Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On Fri, 2017-06-09 at 21:24 +0200, Bjørn Mork wrote: > Chenbo Feng writes: > > > This patch is still under working since it may have problem with > > ip_fragment() call, did you applied it already? Should I send a revert > > patch to you then? > > It does? I initially thought so too, but looking closer I believe the > ip6_copy_metadata() calls in ip6_fragment() takes care of it. Indeed, this should be fine.
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On Fri, 2017-06-09 at 16:12 -0700, Chenbo Feng wrote: > > On 06/09/2017 12:39 PM, David Miller wrote: > > From: Chenbo Feng > > Date: Fri, 9 Jun 2017 12:13:57 -0700 > > > >> > >> On 06/09/2017 12:08 PM, David Miller wrote: > >>> From: Chenbo Feng > >>> Date: Fri, 9 Jun 2017 12:06:07 -0700 > >>> > From: Chenbo Feng > > Move the initialization of skb->dev and skb->protocol from > ip6_finish_output2 to ip6_output. This can make the skb->dev and > skb->protocol information avalaible to the CGROUP eBPF filter. > > Signed-off-by: Chenbo Feng > Acked-by: Eric Dumazet > >>> Applied, thanks. > >>> > >>> This makes ipv6 consistent with ipv4. > >>> > >>> I am surprised this wasn't noticed, for example, in netfilter. > >>> . > >>> > >> Hi David, > >> > >> This patch is still under working since it may have problem with > >> ip_fragment() call, did you applied it already? Should I send a revert > >> patch to you then? > > A revert is necessary or a relative fixup. > > > > Thank you. > > > Hi David, > > The revert is uploaded here: http://patchwork.ozlabs.org/patch/774136/ > > Thanks and sorry for the trouble caused > > Chenbo Feng No worries ! It seems the revert is not needed, after further analysis. One of the point I raised was this no longer needed chunk, that can be added as a separate patch : diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 02cd44f0953900108701895108b2fdaa9f9980e5..0d6f3b6345de26c329ae1d6f25dde652a5452d4b 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -869,7 +869,6 @@ int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, if (skb->sk && dst_allfrag(skb_dst(skb))) sk_nocaps_add(skb->sk, NETIF_F_GSO_MASK); - skb->dev = skb_dst(skb)->dev; icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu); err = -EMSGSIZE;
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On 06/09/2017 12:39 PM, David Miller wrote: From: Chenbo Feng Date: Fri, 9 Jun 2017 12:13:57 -0700 On 06/09/2017 12:08 PM, David Miller wrote: From: Chenbo Feng Date: Fri, 9 Jun 2017 12:06:07 -0700 From: Chenbo Feng Move the initialization of skb->dev and skb->protocol from ip6_finish_output2 to ip6_output. This can make the skb->dev and skb->protocol information avalaible to the CGROUP eBPF filter. Signed-off-by: Chenbo Feng Acked-by: Eric Dumazet Applied, thanks. This makes ipv6 consistent with ipv4. I am surprised this wasn't noticed, for example, in netfilter. . Hi David, This patch is still under working since it may have problem with ip_fragment() call, did you applied it already? Should I send a revert patch to you then? A revert is necessary or a relative fixup. Thank you. Hi David, The revert is uploaded here: http://patchwork.ozlabs.org/patch/774136/ Thanks and sorry for the trouble caused Chenbo Feng
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On Fri, Jun 9, 2017 at 12:06 PM, Chenbo Feng wrote: > From: Chenbo Feng > > Move the initialization of skb->dev and skb->protocol from > ip6_finish_output2 to ip6_output. This can make the skb->dev and > skb->protocol information avalaible to the CGROUP eBPF filter. > > Signed-off-by: Chenbo Feng > Acked-by: Eric Dumazet > --- Arg, you mixed my Acked-by for your other patch :/
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On 06/09/2017 12:24 PM, Bjørn Mork wrote: Chenbo Feng writes: This patch is still under working since it may have problem with ip_fragment() call, did you applied it already? Should I send a revert patch to you then? It does? I initially thought so too, but looking closer I believe the ip6_copy_metadata() calls in ip6_fragment() takes care of it. Bjørn At least in the fail_toobig code path of ip_fragment() call, skb->dev get assigned again. It seems to be redundant with this patch or it will rewrite the skb->dev field. I will revert this one and upload again after I have a proper handle for that.
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng Date: Fri, 9 Jun 2017 12:13:57 -0700 > > > On 06/09/2017 12:08 PM, David Miller wrote: >> From: Chenbo Feng >> Date: Fri, 9 Jun 2017 12:06:07 -0700 >> >>> From: Chenbo Feng >>> >>> Move the initialization of skb->dev and skb->protocol from >>> ip6_finish_output2 to ip6_output. This can make the skb->dev and >>> skb->protocol information avalaible to the CGROUP eBPF filter. >>> >>> Signed-off-by: Chenbo Feng >>> Acked-by: Eric Dumazet >> Applied, thanks. >> >> This makes ipv6 consistent with ipv4. >> >> I am surprised this wasn't noticed, for example, in netfilter. >> . >> > Hi David, > > This patch is still under working since it may have problem with > ip_fragment() call, did you applied it already? Should I send a revert > patch to you then? A revert is necessary or a relative fixup. Thank you.
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng Date: Fri, 9 Jun 2017 12:08:39 -0700 > Sorry, this is the wrong patch, please ignore it. :-/ already applied it. You must now send a relative fixup patch.
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
Chenbo Feng writes: > This patch is still under working since it may have problem with > ip_fragment() call, did you applied it already? Should I send a revert > patch to you then? It does? I initially thought so too, but looking closer I believe the ip6_copy_metadata() calls in ip6_fragment() takes care of it. Bjørn
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
On 06/09/2017 12:08 PM, David Miller wrote: From: Chenbo Feng Date: Fri, 9 Jun 2017 12:06:07 -0700 From: Chenbo Feng Move the initialization of skb->dev and skb->protocol from ip6_finish_output2 to ip6_output. This can make the skb->dev and skb->protocol information avalaible to the CGROUP eBPF filter. Signed-off-by: Chenbo Feng Acked-by: Eric Dumazet Applied, thanks. This makes ipv6 consistent with ipv4. I am surprised this wasn't noticed, for example, in netfilter. . Hi David, This patch is still under working since it may have problem with ip_fragment() call, did you applied it already? Should I send a revert patch to you then? Chenbo Feng
Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng Date: Fri, 9 Jun 2017 12:06:07 -0700 > From: Chenbo Feng > > Move the initialization of skb->dev and skb->protocol from > ip6_finish_output2 to ip6_output. This can make the skb->dev and > skb->protocol information avalaible to the CGROUP eBPF filter. > > Signed-off-by: Chenbo Feng > Acked-by: Eric Dumazet Applied, thanks. This makes ipv6 consistent with ipv4. I am surprised this wasn't noticed, for example, in netfilter.
[PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng
Move the initialization of skb->dev and skb->protocol from
ip6_finish_output2 to ip6_output. This can make the skb->dev and
skb->protocol information avalaible to the CGROUP eBPF filter.
Signed-off-by: Chenbo Feng
Acked-by: Eric Dumazet
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index bf8a58a..02cd44f 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -67,9 +67,6 @@ static int ip6_finish_output2(struct net *net, struct sock
*sk, struct sk_buff *
struct in6_addr *nexthop;
int ret;
- skb->protocol = htons(ETH_P_IPV6);
- skb->dev = dev;
-
if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) {
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
@@ -154,6 +151,9 @@ int ip6_output(struct net *net, struct sock *sk, struct
sk_buff *skb)
struct net_device *dev = skb_dst(skb)->dev;
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->dev = dev;
+
if (unlikely(idev->cnf.disable_ipv6)) {
IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
kfree_skb(skb);
--
2.7.4
