On 3/25/19 3:17 AM, Sukumar Gopalakrishnan wrote:
> 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.
> 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, struct
> sk_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
>
