Message ID | 20190530050815.20352-1-ssuryaextr@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net] vrf: Increment Icmp6InMsgs on the original netdev | expand |
From: Stephen Suryaputra <ssuryaextr@gmail.com> Date: Thu, 30 May 2019 01:08:15 -0400 > Get the ingress interface and increment ICMP counters based on that > instead of skb->dev when the the dev is a VRF device. > > This is a follow up on the following message: > https://www.spinics.net/lists/netdev/msg560268.html > > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com> David, please review.
On 5/29/19 11:08 PM, Stephen Suryaputra wrote: > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c > index 1a832f5e190b..9b365c345c34 100644 > --- a/net/ipv6/reassembly.c > +++ b/net/ipv6/reassembly.c > @@ -260,6 +260,9 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, > int payload_len; > u8 ecn; > > + if (netif_is_l3_master(dev)) > + dev = dev_get_by_index_rcu(net, inet6_iif(skb)); > + > inet_frag_kill(&fq->q); > > ecn = ip_frag_ecn_table[fq->ecn]; > this part changes skb->dev. Seems like it has an unintended effect if the packet is delivered locally.
On Fri, May 31, 2019 at 05:06:16PM -0600, David Ahern wrote: > On 5/29/19 11:08 PM, Stephen Suryaputra wrote: > > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c > > index 1a832f5e190b..9b365c345c34 100644 > > --- a/net/ipv6/reassembly.c > > +++ b/net/ipv6/reassembly.c > > @@ -260,6 +260,9 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, > > int payload_len; > > u8 ecn; > > > > + if (netif_is_l3_master(dev)) > > + dev = dev_get_by_index_rcu(net, inet6_iif(skb)); > > + > > inet_frag_kill(&fq->q); > > > > ecn = ip_frag_ecn_table[fq->ecn]; > > > > this part changes skb->dev. Seems like it has an unintended effect if > the packet is delivered locally. Ah, right. How about this then? +/** + * __in6_dev_stats_get - get inet6_dev pointer for stats + * @dev: network device + * @skb: skb for original incoming interface if neeeded + * + * Caller must hold rcu_read_lock or RTNL, because this function + * does not take a reference on the inet6_dev. + */ +static inline struct inet6_dev *__in6_dev_stats_get(const struct net_device *dev, + const struct sk_buff *skb) +{ + if (netif_is_l3_master(dev)) + dev = dev_get_by_index_rcu(dev_net(dev), inet6_iif(skb)); + return __in6_dev_get(dev); +} @@ -260,9 +260,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, int payload_len; u8 ecn; - if (netif_is_l3_master(dev)) - dev = dev_get_by_index_rcu(net, inet6_iif(skb)); - inet_frag_kill(&fq->q); ecn = ip_frag_ecn_table[fq->ecn]; @@ -305,7 +302,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, skb_network_header_len(skb)); rcu_read_lock(); - __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS); + __IP6_INC_STATS(net, __in6_dev_stats_get(dev, skb), IPSTATS_MIB_REASMOKS); rcu_read_unlock(); fq->q.rb_fragments = RB_ROOT; fq->q.fragments_tail = NULL; @@ -319,7 +316,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, net_dbg_ratelimited("ip6_frag_reasm: no memory for reassembly\n"); out_fail: rcu_read_lock(); - __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS); + __IP6_INC_STATS(net, __in6_dev_stats_get(dev, skb), IPSTATS_MIB_REASMFAILS); rcu_read_unlock(); inet_frag_kill(&fq->q); return -1; Thanks.
On 6/1/19 12:14 PM, Stephen Suryaputra wrote: > On Fri, May 31, 2019 at 05:06:16PM -0600, David Ahern wrote: >> On 5/29/19 11:08 PM, Stephen Suryaputra wrote: >>> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c >>> index 1a832f5e190b..9b365c345c34 100644 >>> --- a/net/ipv6/reassembly.c >>> +++ b/net/ipv6/reassembly.c >>> @@ -260,6 +260,9 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, >>> int payload_len; >>> u8 ecn; >>> >>> + if (netif_is_l3_master(dev)) >>> + dev = dev_get_by_index_rcu(net, inet6_iif(skb)); >>> + >>> inet_frag_kill(&fq->q); >>> >>> ecn = ip_frag_ecn_table[fq->ecn]; >>> >> >> this part changes skb->dev. Seems like it has an unintended effect if >> the packet is delivered locally. > > Ah, right. How about this then? > looks ok to me.
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c index 802faa2fcc0e..f54191cd1d7b 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c @@ -398,23 +398,28 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net, return ERR_PTR(err); } -static int icmp6_iif(const struct sk_buff *skb) +static struct net_device *icmp6_dev(const struct sk_buff *skb) { - int iif = skb->dev->ifindex; + struct net_device *dev = skb->dev; /* for local traffic to local address, skb dev is the loopback * device. Check if there is a dst attached to the skb and if so * get the real device index. Same is needed for replies to a link * local address on a device enslaved to an L3 master device */ - if (unlikely(iif == LOOPBACK_IFINDEX || netif_is_l3_master(skb->dev))) { + if (unlikely(dev->ifindex == LOOPBACK_IFINDEX || netif_is_l3_master(skb->dev))) { const struct rt6_info *rt6 = skb_rt6_info(skb); if (rt6) - iif = rt6->rt6i_idev->dev->ifindex; + dev = rt6->rt6i_idev->dev; } - return iif; + return dev; +} + +static int icmp6_iif(const struct sk_buff *skb) +{ + return icmp6_dev(skb)->ifindex; } /* @@ -801,7 +806,7 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info) static int icmpv6_rcv(struct sk_buff *skb) { struct net *net = dev_net(skb->dev); - struct net_device *dev = skb->dev; + struct net_device *dev = icmp6_dev(skb); struct inet6_dev *idev = __in6_dev_get(dev); const struct in6_addr *saddr, *daddr; struct icmp6hdr *hdr; diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c index 1a832f5e190b..9b365c345c34 100644 --- a/net/ipv6/reassembly.c +++ b/net/ipv6/reassembly.c @@ -260,6 +260,9 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, int payload_len; u8 ecn; + if (netif_is_l3_master(dev)) + dev = dev_get_by_index_rcu(net, inet6_iif(skb)); + inet_frag_kill(&fq->q); ecn = ip_frag_ecn_table[fq->ecn];
Get the ingress interface and increment ICMP counters based on that instead of skb->dev when the the dev is a VRF device. This is a follow up on the following message: https://www.spinics.net/lists/netdev/msg560268.html Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com> --- net/ipv6/icmp.c | 17 +++++++++++------ net/ipv6/reassembly.c | 3 +++ 2 files changed, 14 insertions(+), 6 deletions(-)