From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [PATCH net-next 4/5] ipv6: do not drop vrf udp multicast packets Date: Thu, 20 Sep 2018 15:02:25 +0200 Message-ID: <09797df92f76885b0e3244a3aca68dbf07a09a3a.camel@redhat.com> References: <20180920085848.17721-1-mmanning@vyatta.att-mail.com> <20180920085848.17721-5-mmanning@vyatta.att-mail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Dewi Morgan To: Mike Manning , netdev@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:37104 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727078AbeITSpv (ORCPT ); Thu, 20 Sep 2018 14:45:51 -0400 In-Reply-To: <20180920085848.17721-5-mmanning@vyatta.att-mail.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi, On Thu, 2018-09-20 at 09:58 +0100, Mike Manning wrote: > diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c > index 108f5f88ec98..fc60f297d95b 100644 > --- a/net/ipv6/ip6_input.c > +++ b/net/ipv6/ip6_input.c > @@ -325,9 +325,12 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk > { > const struct inet6_protocol *ipprot; > struct inet6_dev *idev; > + struct net_device *dev; > unsigned int nhoff; > + int sdif = inet6_sdif(skb); > int nexthdr; > bool raw; > + bool deliver; > bool have_final = false; Please, try instead to sort the variable in reverse x-mas tree order. > > /* > @@ -371,9 +374,27 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk > skb_postpull_rcsum(skb, skb_network_header(skb), > skb_network_header_len(skb)); > hdr = ipv6_hdr(skb); > - if (ipv6_addr_is_multicast(&hdr->daddr) && > - !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr, > - &hdr->saddr) && > + > + /* skb->dev passed may be master dev for vrfs. */ > + if (sdif) { > + rcu_read_lock(); AFAICS, the rcu lock is already acquired at the beginning of ip6_input_finish(), not need to acquire it here again. + dev = dev_get_by_index_rcu(dev_net(skb->dev), > + sdif); > + if (!dev) { > + rcu_read_unlock(); > + kfree_skb(skb); > + return -ENODEV; > + } > + } else { > + dev = skb->dev; The above fragment of code is a recurring pattern in this series, perhaps adding an helper for it would reduce code duplication ? Cheers, Paolo