Re: VRF IPV6: Icmp6InMsgs incremented on VRF device instead of ingress VLAN device.
From: David Ahern <hidden>
Date: 2019-03-26 14:45:54
On 3/25/19 3:17 AM, Sukumar Gopalakrishnan wrote:
quoted hunk
PROPOSED FIX: ============= skb->dev is pointing to vrf device (vrf_258) for ingressing IPV6 global address instead of VLAN interface . Get the ingress interface from IP6CB(head)->iif and increment ICMP counters on the VLAN interface instead of VRF device.diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c index 63225d1405e3..f4889a14c087 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c@@ -806,7 +806,7 @@ out:static int icmpv6_rcv(struct sk_buff *skb) { - struct net_device *dev = skb->dev; + struct net_device *dev = dev_get_by_index_rcu(&init_net, icmp6_iif(skb));
you don't need to go from device to index to device and init_net is not
always right. I think something like this would be better:
static struct net_device *icmp6_dev(const struct sk_buff *skb)
{
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(dev->ifindex == LOOPBACK_IFINDEX ||
netif_is_l3_master(skb->dev))) {
const struct rt6_info *rt6 = skb_rt6_info(skb);
if (rt6)
dev = rt6->rt6i_idev->dev;
}
return dev;
}
static int icmp6_iif(const struct sk_buff *skb)
{
return icmp6_dev(skb)->ifindex;
}
and the you change icmpv6_rcv():
struct net_device *dev = icmp6_dev(skb);
that avoids the dev to index to dev and the need for a namespace reference.
quoted hunk
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 846012eae526..b18f7901b1c3 100644 --- a/net/ipv6/reassembly.c +++ b/net/ipv6/reassembly.c@@ -506,7 +506,8 @@ static int ip6_frag_reasm(struct frag_queue *fq, structsk_buff *prev, skb_network_header_len(head)); rcu_read_lock(); - __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS); + __IP6_INC_STATS(net, __in6_dev_get((dev_get_by_index_rcu(net, + IP6CB(head)->iif))), IPSTATS_MIB_REASMOKS);
and here I think this is simpler if you just update dev: dev = dev_get_by_index_rcu(net, IP6CB(head)->iif); and leave the __IP6_INC_STATS as is. Also, I think that should be inet6_iif(head) and not IP6CB(head)->iif.
rcu_read_unlock(); fq->q.fragments = NULL; fq->q.fragments_tail = NULL; Ingress device's counter is getting incremented correctly. /proc/2429/net/dev_snmp6 # cat v6_vl_F4253 | grep Icmp6InMsgs Icmp6InMsgs 1540 /proc/2429/net/dev_snmp6 # cat vrf_258 | grep Icmp6InMsgs Icmp6InMsgs 0