* [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs
@ 2021-05-25 9:13 Paul Blakey
2021-05-25 13:45 ` Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paul Blakey @ 2021-05-25 9:13 UTC (permalink / raw)
To: Paul Blakey, netdev, David S. Miller, Jakub Kicinski
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Saeed Mahameed, Oz Shlomo, Roi Dayan, Vlad Buslov
When multiple SKBs are merged to a new skb under napi GRO,
or SKB is re-used by napi, if nfct was set for them in the
driver, it will not be released while freeing their stolen
head state or on re-use.
Release nfct on napi's stolen or re-used SKBs.
Fixes: 5c6b94604744 ("net/mlx5e: CT: Handle misses after executing CT action")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Paul Blakey <paulb@nvidia.com>
---
net/core/dev.c | 1 +
net/core/skbuff.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index ef8cf7619baf..a5324ca7dc65 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6243,6 +6243,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
skb_ext_reset(skb);
+ nf_reset_ct(skb);
napi->skb = skb;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3ad22870298c..6127bab2fe2f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -939,6 +939,7 @@ void __kfree_skb_defer(struct sk_buff *skb)
void napi_skb_free_stolen_head(struct sk_buff *skb)
{
+ nf_conntrack_put(skb_nfct(skb));
skb_dst_drop(skb);
skb_ext_put(skb);
napi_skb_cache_put(skb);
--
2.30.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs
2021-05-25 9:13 [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs Paul Blakey
@ 2021-05-25 13:45 ` Eric Dumazet
2021-05-27 13:30 ` Paul Blakey
2021-05-25 14:47 ` kernel test robot
2021-05-25 14:50 ` kernel test robot
2 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2021-05-25 13:45 UTC (permalink / raw)
To: Paul Blakey, netdev, David S. Miller, Jakub Kicinski
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Saeed Mahameed, Oz Shlomo, Roi Dayan, Vlad Buslov
On 5/25/21 11:13 AM, Paul Blakey wrote:
> When multiple SKBs are merged to a new skb under napi GRO,
> or SKB is re-used by napi, if nfct was set for them in the
> driver, it will not be released while freeing their stolen
> head state or on re-use.
>
> Release nfct on napi's stolen or re-used SKBs.
>
> Fixes: 5c6b94604744 ("net/mlx5e: CT: Handle misses after executing CT action")
> Reviewed-by: Roi Dayan <roid@nvidia.com>
> Signed-off-by: Paul Blakey <paulb@nvidia.com>
> ---
> net/core/dev.c | 1 +
> net/core/skbuff.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ef8cf7619baf..a5324ca7dc65 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6243,6 +6243,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
> skb_shinfo(skb)->gso_type = 0;
> skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
> skb_ext_reset(skb);
> + nf_reset_ct(skb);
>
> napi->skb = skb;
> }
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 3ad22870298c..6127bab2fe2f 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -939,6 +939,7 @@ void __kfree_skb_defer(struct sk_buff *skb)
>
> void napi_skb_free_stolen_head(struct sk_buff *skb)
> {
> + nf_conntrack_put(skb_nfct(skb));
> skb_dst_drop(skb);
> skb_ext_put(skb);
> napi_skb_cache_put(skb);
>
Sadly we are very consistently making GRO slow as hell.
Why merging SKB with different ct would be allowed ?
If we accept this patch, then you will likely add another check in gro_list_prepare() ?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs
2021-05-25 13:45 ` Eric Dumazet
@ 2021-05-27 13:30 ` Paul Blakey
0 siblings, 0 replies; 7+ messages in thread
From: Paul Blakey @ 2021-05-27 13:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, John Fastabend, Saeed Mahameed, Oz Shlomo,
Roi Dayan, Vlad Buslov
On Tue, 25 May 2021, Eric Dumazet wrote:
>
>
> On 5/25/21 11:13 AM, Paul Blakey wrote:
> > When multiple SKBs are merged to a new skb under napi GRO,
> > or SKB is re-used by napi, if nfct was set for them in the
> > driver, it will not be released while freeing their stolen
> > head state or on re-use.
> >
> > Release nfct on napi's stolen or re-used SKBs.
> >
> > Fixes: 5c6b94604744 ("net/mlx5e: CT: Handle misses after executing CT action")
> > Reviewed-by: Roi Dayan <roid@nvidia.com>
> > Signed-off-by: Paul Blakey <paulb@nvidia.com>
> > ---
> > net/core/dev.c | 1 +
> > net/core/skbuff.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index ef8cf7619baf..a5324ca7dc65 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6243,6 +6243,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
> > skb_shinfo(skb)->gso_type = 0;
> > skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
> > skb_ext_reset(skb);
> > + nf_reset_ct(skb);
> >
> > napi->skb = skb;
> > }
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 3ad22870298c..6127bab2fe2f 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -939,6 +939,7 @@ void __kfree_skb_defer(struct sk_buff *skb)
> >
> > void napi_skb_free_stolen_head(struct sk_buff *skb)
> > {
> > + nf_conntrack_put(skb_nfct(skb));
> > skb_dst_drop(skb);
> > skb_ext_put(skb);
> > napi_skb_cache_put(skb);
> >
>
> Sadly we are very consistently making GRO slow as hell.
>
> Why merging SKB with different ct would be allowed ?
>
> If we accept this patch, then you will likely add another check in gro_list_prepare() ?
>
>
Yes, good catch, I'll check if that's even possible in our case.
If so I'm going to add checking diff conntracks on the gro list,
and diff tc chains, both are metadata on the skb.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs
2021-05-25 9:13 [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs Paul Blakey
@ 2021-05-25 14:47 ` kernel test robot
2021-05-25 14:47 ` kernel test robot
2021-05-25 14:50 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-25 14:47 UTC (permalink / raw)
To: Paul Blakey, netdev, David S. Miller, Jakub Kicinski
Cc: kbuild-all, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Saeed Mahameed, Oz Shlomo, Roi Dayan
[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Paul-Blakey/skbuff-Release-nfct-refcount-on-napi-stolen-or-re-used-skbs/20210525-171507
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 8c42a49738f16af0061f9ae5c2f5a955f268d9e3
config: i386-randconfig-r005-20210525 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/d4340b4d15d7b5f156b533d1ab0b3ae75ed479d3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Paul-Blakey/skbuff-Release-nfct-refcount-on-napi-stolen-or-re-used-skbs/20210525-171507
git checkout d4340b4d15d7b5f156b533d1ab0b3ae75ed479d3
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/core/skbuff.c: In function 'napi_skb_free_stolen_head':
>> net/core/skbuff.c:942:2: error: implicit declaration of function 'nf_conntrack_put' [-Werror=implicit-function-declaration]
942 | nf_conntrack_put(skb_nfct(skb));
| ^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/nf_conntrack_put +942 net/core/skbuff.c
939
940 void napi_skb_free_stolen_head(struct sk_buff *skb)
941 {
> 942 nf_conntrack_put(skb_nfct(skb));
943 skb_dst_drop(skb);
944 skb_ext_put(skb);
945 napi_skb_cache_put(skb);
946 }
947
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32848 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs
2021-05-25 9:13 [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs Paul Blakey
@ 2021-05-25 14:50 ` kernel test robot
2021-05-25 14:47 ` kernel test robot
2021-05-25 14:50 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-05-25 14:50 UTC (permalink / raw)
To: Paul Blakey, netdev, David S. Miller, Jakub Kicinski
Cc: kbuild-all, clang-built-linux, Alexei Starovoitov,
Daniel Borkmann, John Fastabend, Saeed Mahameed, Oz Shlomo,
Roi Dayan
[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Paul-Blakey/skbuff-Release-nfct-refcount-on-napi-stolen-or-re-used-skbs/20210525-171507
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 8c42a49738f16af0061f9ae5c2f5a955f268d9e3
config: x86_64-randconfig-r032-20210525 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/d4340b4d15d7b5f156b533d1ab0b3ae75ed479d3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Paul-Blakey/skbuff-Release-nfct-refcount-on-napi-stolen-or-re-used-skbs/20210525-171507
git checkout d4340b4d15d7b5f156b533d1ab0b3ae75ed479d3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> net/core/skbuff.c:942:2: error: implicit declaration of function 'nf_conntrack_put' [-Werror,-Wimplicit-function-declaration]
nf_conntrack_put(skb_nfct(skb));
^
1 error generated.
vim +/nf_conntrack_put +942 net/core/skbuff.c
939
940 void napi_skb_free_stolen_head(struct sk_buff *skb)
941 {
> 942 nf_conntrack_put(skb_nfct(skb));
943 skb_dst_drop(skb);
944 skb_ext_put(skb);
945 napi_skb_cache_put(skb);
946 }
947
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34135 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-27 13:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 9:13 [PATCH net] skbuff: Release nfct refcount on napi stolen or re-used skbs Paul Blakey
2021-05-25 13:45 ` Eric Dumazet
2021-05-27 13:30 ` Paul Blakey
2021-05-25 14:47 ` kernel test robot
2021-05-25 14:47 ` kernel test robot
2021-05-25 14:50 ` kernel test robot
2021-05-25 14:50 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.