netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net-core: add InMacErrors counter
@ 2022-01-22  0:03 Jeffrey Ji
  2022-01-22  3:40 ` Jakub Kicinski
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jeffrey Ji @ 2022-01-22  0:03 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: Brian Vazquez, linux-kernel, netdev, jeffreyjilinux, jeffreyji

From: jeffreyji <jeffreyji@google.com>

Increment InMacErrors counter when packet dropped due to incorrect dest
MAC addr.

example output from nstat:
\~# nstat -z "*InMac*"
\#kernel
Ip6InMacErrors                  0                  0.0
IpExtInMacErrors                1                  0.0

Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
counter was incremented.

Signed-off-by: jeffreyji <jeffreyji@google.com>
---
 include/uapi/linux/snmp.h | 1 +
 net/ipv4/ip_input.c       | 4 +++-
 net/ipv4/proc.c           | 1 +
 net/ipv6/ip6_input.c      | 1 +
 net/ipv6/proc.c           | 1 +
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 904909d020e2..ac2fac12dd7d 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -57,6 +57,7 @@ enum
 	IPSTATS_MIB_ECT0PKTS,			/* InECT0Pkts */
 	IPSTATS_MIB_CEPKTS,			/* InCEPkts */
 	IPSTATS_MIB_REASM_OVERLAPS,		/* ReasmOverlaps */
+	IPSTATS_MIB_INMACERRORS,		/* InMacErrors */
 	__IPSTATS_MIB_MAX
 };
 
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 3a025c011971..0961585f7c02 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -441,8 +441,10 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
 	/* When the interface is in promisc. mode, drop all the crap
 	 * that it receives, do not try to analyse it.
 	 */
-	if (skb->pkt_type == PACKET_OTHERHOST)
+	if (skb->pkt_type == PACKET_OTHERHOST) {
+		__IP_INC_STATS(net, IPSTATS_MIB_INMACERRORS);
 		goto drop;
+	}
 
 	__IP_UPD_PO_STATS(net, IPSTATS_MIB_IN, skb->len);
 
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index f30273afb539..dfe0a1dbf8e9 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -117,6 +117,7 @@ static const struct snmp_mib snmp4_ipextstats_list[] = {
 	SNMP_MIB_ITEM("InECT0Pkts", IPSTATS_MIB_ECT0PKTS),
 	SNMP_MIB_ITEM("InCEPkts", IPSTATS_MIB_CEPKTS),
 	SNMP_MIB_ITEM("ReasmOverlaps", IPSTATS_MIB_REASM_OVERLAPS),
+	SNMP_MIB_ITEM("InMacErrors", IPSTATS_MIB_INMACERRORS),
 	SNMP_MIB_SENTINEL
 };
 
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 80256717868e..2903869274ca 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -150,6 +150,7 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
 	struct inet6_dev *idev;
 
 	if (skb->pkt_type == PACKET_OTHERHOST) {
+		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INMACERRORS);
 		kfree_skb(skb);
 		return NULL;
 	}
diff --git a/net/ipv6/proc.c b/net/ipv6/proc.c
index d6306aa46bb1..76e6119ba558 100644
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -84,6 +84,7 @@ static const struct snmp_mib snmp6_ipstats_list[] = {
 	SNMP_MIB_ITEM("Ip6InECT1Pkts", IPSTATS_MIB_ECT1PKTS),
 	SNMP_MIB_ITEM("Ip6InECT0Pkts", IPSTATS_MIB_ECT0PKTS),
 	SNMP_MIB_ITEM("Ip6InCEPkts", IPSTATS_MIB_CEPKTS),
+	SNMP_MIB_ITEM("Ip6InMacErrors", IPSTATS_MIB_INMACERRORS),
 	SNMP_MIB_SENTINEL
 };
 
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-22  0:03 [PATCH net-next] net-core: add InMacErrors counter Jeffrey Ji
@ 2022-01-22  3:40 ` Jakub Kicinski
  2022-01-24 17:13   ` Brian Vazquez
  2022-01-24 22:24 ` kernel test robot
  2022-01-24 22:24 ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2022-01-22  3:40 UTC (permalink / raw)
  To: Jeffrey Ji
  Cc: Eric Dumazet, David S . Miller, Brian Vazquez, linux-kernel,
	netdev, jeffreyji

On Sat, 22 Jan 2022 00:03:01 +0000 Jeffrey Ji wrote:
> From: jeffreyji <jeffreyji@google.com>
> 
> Increment InMacErrors counter when packet dropped due to incorrect dest
> MAC addr.
> 
> example output from nstat:
> \~# nstat -z "*InMac*"
> \#kernel
> Ip6InMacErrors                  0                  0.0
> IpExtInMacErrors                1                  0.0
> 
> Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
> counter was incremented.
> 
> Signed-off-by: jeffreyji <jeffreyji@google.com>

How about we use the new kfree_skb_reason() instead to avoid allocating
per-netns memory the stats?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-22  3:40 ` Jakub Kicinski
@ 2022-01-24 17:13   ` Brian Vazquez
  2022-01-24 17:29     ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Brian Vazquez @ 2022-01-24 17:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jeffrey Ji, Eric Dumazet, David S . Miller, linux-kernel, netdev,
	jeffreyji

On Fri, Jan 21, 2022 at 7:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 22 Jan 2022 00:03:01 +0000 Jeffrey Ji wrote:
> > From: jeffreyji <jeffreyji@google.com>
> >
> > Increment InMacErrors counter when packet dropped due to incorrect dest
> > MAC addr.
> >
> > example output from nstat:
> > \~# nstat -z "*InMac*"
> > \#kernel
> > Ip6InMacErrors                  0                  0.0
> > IpExtInMacErrors                1                  0.0
> >
> > Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> > with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
> > counter was incremented.
> >
> > Signed-off-by: jeffreyji <jeffreyji@google.com>
>
> How about we use the new kfree_skb_reason() instead to avoid allocating
> per-netns memory the stats?

I'm not too familiar with the new kfree_skb_reason , but my
understanding is that it needs either the drop_monitor  or ebpf to get
the reason from the tracepoint, right? This is not too different from
using perf tool to find where the pkt is being dropped.

The idea here was to have a high level metric that is easier to find
for users that have less expertise on using more advance tools.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-24 17:13   ` Brian Vazquez
@ 2022-01-24 17:29     ` Jakub Kicinski
  2022-01-24 17:39       ` Eric Dumazet
  2022-01-24 20:19       ` Cong Wang
  0 siblings, 2 replies; 10+ messages in thread
From: Jakub Kicinski @ 2022-01-24 17:29 UTC (permalink / raw)
  To: Brian Vazquez
  Cc: Jeffrey Ji, Eric Dumazet, David S . Miller, linux-kernel, netdev,
	jeffreyji

On Mon, 24 Jan 2022 09:13:12 -0800 Brian Vazquez wrote:
> On Fri, Jan 21, 2022 at 7:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Sat, 22 Jan 2022 00:03:01 +0000 Jeffrey Ji wrote:  
> > > From: jeffreyji <jeffreyji@google.com>
> > >
> > > Increment InMacErrors counter when packet dropped due to incorrect dest
> > > MAC addr.
> > >
> > > example output from nstat:
> > > \~# nstat -z "*InMac*"
> > > \#kernel
> > > Ip6InMacErrors                  0                  0.0
> > > IpExtInMacErrors                1                  0.0
> > >
> > > Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> > > with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
> > > counter was incremented.
> > >
> > > Signed-off-by: jeffreyji <jeffreyji@google.com>  
> >
> > How about we use the new kfree_skb_reason() instead to avoid allocating
> > per-netns memory the stats?  
> 
> I'm not too familiar with the new kfree_skb_reason , but my
> understanding is that it needs either the drop_monitor  or ebpf to get
> the reason from the tracepoint, right? This is not too different from
> using perf tool to find where the pkt is being dropped.
> 
> The idea here was to have a high level metric that is easier to find
> for users that have less expertise on using more advance tools.

That much it's understood, but it's a trade off. This drop point
existed for 20 years, why do we need to consume extra memory now?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-24 17:29     ` Jakub Kicinski
@ 2022-01-24 17:39       ` Eric Dumazet
  2022-01-24 17:46         ` Jakub Kicinski
  2022-01-24 20:19       ` Cong Wang
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2022-01-24 17:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Brian Vazquez, Jeffrey Ji, David S . Miller, LKML, netdev, jeffreyji

On Mon, Jan 24, 2022 at 9:29 AM Jakub Kicinski <kuba@kernel.org> wrote:

> That much it's understood, but it's a trade off. This drop point
> existed for 20 years, why do we need to consume extra memory now?

Being able to tell after the facts, that such a drop reason had
occured can help investigations.

nstat -a | grep InMac

Jeffrey, what about _also_ adding the kfree_skb_reason(), since this
seems to also help ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-24 17:39       ` Eric Dumazet
@ 2022-01-24 17:46         ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2022-01-24 17:46 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Brian Vazquez, Jeffrey Ji, David S . Miller, LKML, netdev, jeffreyji

On Mon, 24 Jan 2022 09:39:22 -0800 Eric Dumazet wrote:
> On Mon, Jan 24, 2022 at 9:29 AM Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > That much it's understood, but it's a trade off. This drop point
> > existed for 20 years, why do we need to consume extra memory now?  
> 
> Being able to tell after the facts, that such a drop reason had
> occured can help investigations.
> 
> nstat -a | grep InMac
> 
> Jeffrey, what about _also_ adding the kfree_skb_reason(), since this
> seems to also help ?

Yes, and please add a proper justification to the commit message, 
with real life examples of cases where this drop point has proven
to be the culprit. Right now the commit message says gives the example
of trafgen :(


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-24 17:29     ` Jakub Kicinski
  2022-01-24 17:39       ` Eric Dumazet
@ 2022-01-24 20:19       ` Cong Wang
  2022-01-24 20:21         ` Eric Dumazet
  1 sibling, 1 reply; 10+ messages in thread
From: Cong Wang @ 2022-01-24 20:19 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Brian Vazquez, Jeffrey Ji, Eric Dumazet, David S . Miller,
	linux-kernel, netdev, jeffreyji

On Mon, Jan 24, 2022 at 09:29:24AM -0800, Jakub Kicinski wrote:
> On Mon, 24 Jan 2022 09:13:12 -0800 Brian Vazquez wrote:
> > On Fri, Jan 21, 2022 at 7:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Sat, 22 Jan 2022 00:03:01 +0000 Jeffrey Ji wrote:  
> > > > From: jeffreyji <jeffreyji@google.com>
> > > >
> > > > Increment InMacErrors counter when packet dropped due to incorrect dest
> > > > MAC addr.
> > > >
> > > > example output from nstat:
> > > > \~# nstat -z "*InMac*"
> > > > \#kernel
> > > > Ip6InMacErrors                  0                  0.0
> > > > IpExtInMacErrors                1                  0.0
> > > >
> > > > Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> > > > with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
> > > > counter was incremented.
> > > >
> > > > Signed-off-by: jeffreyji <jeffreyji@google.com>  
> > >
> > > How about we use the new kfree_skb_reason() instead to avoid allocating
> > > per-netns memory the stats?  
> > 
> > I'm not too familiar with the new kfree_skb_reason , but my
> > understanding is that it needs either the drop_monitor  or ebpf to get
> > the reason from the tracepoint, right? This is not too different from
> > using perf tool to find where the pkt is being dropped.
> > 
> > The idea here was to have a high level metric that is easier to find
> > for users that have less expertise on using more advance tools.
> 
> That much it's understood, but it's a trade off. This drop point
> existed for 20 years, why do we need to consume extra memory now?

kfree_skb_reason() is for tracing and tracing has overhead in
production, which is higher than just a percpu counter.

What memory overhead are you talking about? We have ~37 IP related
SNMP counters, this patch merely adds 1/37 memory overhead. So, what's the
point? :-/

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-24 20:19       ` Cong Wang
@ 2022-01-24 20:21         ` Eric Dumazet
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-01-24 20:21 UTC (permalink / raw)
  To: Cong Wang
  Cc: Jakub Kicinski, Brian Vazquez, Jeffrey Ji, David S . Miller,
	LKML, netdev, jeffreyji

On Mon, Jan 24, 2022 at 12:20 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Jan 24, 2022 at 09:29:24AM -0800, Jakub Kicinski wrote:
> > On Mon, 24 Jan 2022 09:13:12 -0800 Brian Vazquez wrote:
> > > On Fri, Jan 21, 2022 at 7:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > > On Sat, 22 Jan 2022 00:03:01 +0000 Jeffrey Ji wrote:
> > > > > From: jeffreyji <jeffreyji@google.com>
> > > > >
> > > > > Increment InMacErrors counter when packet dropped due to incorrect dest
> > > > > MAC addr.
> > > > >
> > > > > example output from nstat:
> > > > > \~# nstat -z "*InMac*"
> > > > > \#kernel
> > > > > Ip6InMacErrors                  0                  0.0
> > > > > IpExtInMacErrors                1                  0.0
> > > > >
> > > > > Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> > > > > with "{eth(daddr=$INCORRECT_MAC...}", verified that nstat showed the
> > > > > counter was incremented.
> > > > >
> > > > > Signed-off-by: jeffreyji <jeffreyji@google.com>
> > > >
> > > > How about we use the new kfree_skb_reason() instead to avoid allocating
> > > > per-netns memory the stats?
> > >
> > > I'm not too familiar with the new kfree_skb_reason , but my
> > > understanding is that it needs either the drop_monitor  or ebpf to get
> > > the reason from the tracepoint, right? This is not too different from
> > > using perf tool to find where the pkt is being dropped.
> > >
> > > The idea here was to have a high level metric that is easier to find
> > > for users that have less expertise on using more advance tools.
> >
> > That much it's understood, but it's a trade off. This drop point
> > existed for 20 years, why do we need to consume extra memory now?
>
> kfree_skb_reason() is for tracing and tracing has overhead in
> production, which is higher than just a percpu counter.
>
> What memory overhead are you talking about? We have ~37 IP related
> SNMP counters, this patch merely adds 1/37 memory overhead. So, what's the
> point? :-/
>

BTW I just saw that kfree_skb_reason() changes have been proposed
already by Menglong Dong this morning.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-22  0:03 [PATCH net-next] net-core: add InMacErrors counter Jeffrey Ji
  2022-01-22  3:40 ` Jakub Kicinski
@ 2022-01-24 22:24 ` kernel test robot
  2022-01-24 22:24 ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-01-24 22:24 UTC (permalink / raw)
  To: Jeffrey Ji, Eric Dumazet, David S . Miller
  Cc: llvm, kbuild-all, Brian Vazquez, linux-kernel, netdev,
	jeffreyjilinux, jeffreyji

Hi Jeffrey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeffrey-Ji/net-core-add-InMacErrors-counter/20220122-080455
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8aaaf2f3af2ae212428f4db1af34214225f5cec3
config: x86_64-randconfig-a001-20220117 (https://download.01.org/0day-ci/archive/20220125/202201250416.QEl2tmqY-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 7b3d30728816403d1fd73cc5082e9fb761262bce)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f8ea346d278c116f830459bae2a910fdc5e96a35
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeffrey-Ji/net-core-add-InMacErrors-counter/20220122-080455
        git checkout f8ea346d278c116f830459bae2a910fdc5e96a35
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/ipv6/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/ipv6/ip6_input.c:153:24: warning: variable 'idev' is uninitialized when used here [-Wuninitialized]
                   __IP6_INC_STATS(net, idev, IPSTATS_MIB_INMACERRORS);
                                        ^~~~
   include/net/ipv6.h:256:26: note: expanded from macro '__IP6_INC_STATS'
                   _DEVINC(net, ipv6, __, idev, field)
                                          ^~~~
   include/net/ipv6.h:211:29: note: expanded from macro '_DEVINC'
           struct inet6_dev *_idev = (idev);                               \
                                      ^~~~
   net/ipv6/ip6_input.c:150:24: note: initialize the variable 'idev' to silence this warning
           struct inet6_dev *idev;
                                 ^
                                  = NULL
   1 warning generated.


vim +/idev +153 net/ipv6/ip6_input.c

   144	
   145	static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
   146					    struct net *net)
   147	{
   148		const struct ipv6hdr *hdr;
   149		u32 pkt_len;
   150		struct inet6_dev *idev;
   151	
   152		if (skb->pkt_type == PACKET_OTHERHOST) {
 > 153			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INMACERRORS);
   154			kfree_skb(skb);
   155			return NULL;
   156		}
   157	
   158		rcu_read_lock();
   159	
   160		idev = __in6_dev_get(skb->dev);
   161	
   162		__IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_IN, skb->len);
   163	
   164		if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL ||
   165		    !idev || unlikely(idev->cnf.disable_ipv6)) {
   166			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
   167			goto drop;
   168		}
   169	
   170		memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
   171	
   172		/*
   173		 * Store incoming device index. When the packet will
   174		 * be queued, we cannot refer to skb->dev anymore.
   175		 *
   176		 * BTW, when we send a packet for our own local address on a
   177		 * non-loopback interface (e.g. ethX), it is being delivered
   178		 * via the loopback interface (lo) here; skb->dev = loopback_dev.
   179		 * It, however, should be considered as if it is being
   180		 * arrived via the sending interface (ethX), because of the
   181		 * nature of scoping architecture. --yoshfuji
   182		 */
   183		IP6CB(skb)->iif = skb_valid_dst(skb) ? ip6_dst_idev(skb_dst(skb))->dev->ifindex : dev->ifindex;
   184	
   185		if (unlikely(!pskb_may_pull(skb, sizeof(*hdr))))
   186			goto err;
   187	
   188		hdr = ipv6_hdr(skb);
   189	
   190		if (hdr->version != 6)
   191			goto err;
   192	
   193		__IP6_ADD_STATS(net, idev,
   194				IPSTATS_MIB_NOECTPKTS +
   195					(ipv6_get_dsfield(hdr) & INET_ECN_MASK),
   196				max_t(unsigned short, 1, skb_shinfo(skb)->gso_segs));
   197		/*
   198		 * RFC4291 2.5.3
   199		 * The loopback address must not be used as the source address in IPv6
   200		 * packets that are sent outside of a single node. [..]
   201		 * A packet received on an interface with a destination address
   202		 * of loopback must be dropped.
   203		 */
   204		if ((ipv6_addr_loopback(&hdr->saddr) ||
   205		     ipv6_addr_loopback(&hdr->daddr)) &&
   206		    !(dev->flags & IFF_LOOPBACK) &&
   207		    !netif_is_l3_master(dev))
   208			goto err;
   209	
   210		/* RFC4291 Errata ID: 3480
   211		 * Interface-Local scope spans only a single interface on a
   212		 * node and is useful only for loopback transmission of
   213		 * multicast.  Packets with interface-local scope received
   214		 * from another node must be discarded.
   215		 */
   216		if (!(skb->pkt_type == PACKET_LOOPBACK ||
   217		      dev->flags & IFF_LOOPBACK) &&
   218		    ipv6_addr_is_multicast(&hdr->daddr) &&
   219		    IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 1)
   220			goto err;
   221	
   222		/* If enabled, drop unicast packets that were encapsulated in link-layer
   223		 * multicast or broadcast to protected against the so-called "hole-196"
   224		 * attack in 802.11 wireless.
   225		 */
   226		if (!ipv6_addr_is_multicast(&hdr->daddr) &&
   227		    (skb->pkt_type == PACKET_BROADCAST ||
   228		     skb->pkt_type == PACKET_MULTICAST) &&
   229		    idev->cnf.drop_unicast_in_l2_multicast)
   230			goto err;
   231	
   232		/* RFC4291 2.7
   233		 * Nodes must not originate a packet to a multicast address whose scope
   234		 * field contains the reserved value 0; if such a packet is received, it
   235		 * must be silently dropped.
   236		 */
   237		if (ipv6_addr_is_multicast(&hdr->daddr) &&
   238		    IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 0)
   239			goto err;
   240	
   241		/*
   242		 * RFC4291 2.7
   243		 * Multicast addresses must not be used as source addresses in IPv6
   244		 * packets or appear in any Routing header.
   245		 */
   246		if (ipv6_addr_is_multicast(&hdr->saddr))
   247			goto err;
   248	
   249		skb->transport_header = skb->network_header + sizeof(*hdr);
   250		IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
   251	
   252		pkt_len = ntohs(hdr->payload_len);
   253	
   254		/* pkt_len may be zero if Jumbo payload option is present */
   255		if (pkt_len || hdr->nexthdr != NEXTHDR_HOP) {
   256			if (pkt_len + sizeof(struct ipv6hdr) > skb->len) {
   257				__IP6_INC_STATS(net,
   258						idev, IPSTATS_MIB_INTRUNCATEDPKTS);
   259				goto drop;
   260			}
   261			if (pskb_trim_rcsum(skb, pkt_len + sizeof(struct ipv6hdr))) {
   262				__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   263				goto drop;
   264			}
   265			hdr = ipv6_hdr(skb);
   266		}
   267	
   268		if (hdr->nexthdr == NEXTHDR_HOP) {
   269			if (ipv6_parse_hopopts(skb) < 0) {
   270				__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   271				rcu_read_unlock();
   272				return NULL;
   273			}
   274		}
   275	
   276		rcu_read_unlock();
   277	
   278		/* Must drop socket now because of tproxy. */
   279		if (!skb_sk_is_prefetched(skb))
   280			skb_orphan(skb);
   281	
   282		return skb;
   283	err:
   284		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   285	drop:
   286		rcu_read_unlock();
   287		kfree_skb(skb);
   288		return NULL;
   289	}
   290	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next] net-core: add InMacErrors counter
  2022-01-22  0:03 [PATCH net-next] net-core: add InMacErrors counter Jeffrey Ji
  2022-01-22  3:40 ` Jakub Kicinski
  2022-01-24 22:24 ` kernel test robot
@ 2022-01-24 22:24 ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-01-24 22:24 UTC (permalink / raw)
  To: Jeffrey Ji, Eric Dumazet, David S . Miller
  Cc: llvm, kbuild-all, Brian Vazquez, linux-kernel, netdev,
	jeffreyjilinux, jeffreyji

Hi Jeffrey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeffrey-Ji/net-core-add-InMacErrors-counter/20220122-080455
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8aaaf2f3af2ae212428f4db1af34214225f5cec3
config: mips-maltaaprp_defconfig (https://download.01.org/0day-ci/archive/20220125/202201250449.tTXEfxm8-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 7b3d30728816403d1fd73cc5082e9fb761262bce)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/f8ea346d278c116f830459bae2a910fdc5e96a35
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeffrey-Ji/net-core-add-InMacErrors-counter/20220122-080455
        git checkout f8ea346d278c116f830459bae2a910fdc5e96a35
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash net/ipv6/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/ipv6/ip6_input.c:153:24: warning: variable 'idev' is uninitialized when used here [-Wuninitialized]
                   __IP6_INC_STATS(net, idev, IPSTATS_MIB_INMACERRORS);
                                        ^~~~
   include/net/ipv6.h:256:26: note: expanded from macro '__IP6_INC_STATS'
                   _DEVINC(net, ipv6, __, idev, field)
                                          ^~~~
   include/net/ipv6.h:211:29: note: expanded from macro '_DEVINC'
           struct inet6_dev *_idev = (idev);                               \
                                      ^~~~
   net/ipv6/ip6_input.c:150:24: note: initialize the variable 'idev' to silence this warning
           struct inet6_dev *idev;
                                 ^
                                  = NULL
   1 warning generated.


vim +/idev +153 net/ipv6/ip6_input.c

   144	
   145	static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
   146					    struct net *net)
   147	{
   148		const struct ipv6hdr *hdr;
   149		u32 pkt_len;
   150		struct inet6_dev *idev;
   151	
   152		if (skb->pkt_type == PACKET_OTHERHOST) {
 > 153			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INMACERRORS);
   154			kfree_skb(skb);
   155			return NULL;
   156		}
   157	
   158		rcu_read_lock();
   159	
   160		idev = __in6_dev_get(skb->dev);
   161	
   162		__IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_IN, skb->len);
   163	
   164		if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL ||
   165		    !idev || unlikely(idev->cnf.disable_ipv6)) {
   166			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
   167			goto drop;
   168		}
   169	
   170		memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
   171	
   172		/*
   173		 * Store incoming device index. When the packet will
   174		 * be queued, we cannot refer to skb->dev anymore.
   175		 *
   176		 * BTW, when we send a packet for our own local address on a
   177		 * non-loopback interface (e.g. ethX), it is being delivered
   178		 * via the loopback interface (lo) here; skb->dev = loopback_dev.
   179		 * It, however, should be considered as if it is being
   180		 * arrived via the sending interface (ethX), because of the
   181		 * nature of scoping architecture. --yoshfuji
   182		 */
   183		IP6CB(skb)->iif = skb_valid_dst(skb) ? ip6_dst_idev(skb_dst(skb))->dev->ifindex : dev->ifindex;
   184	
   185		if (unlikely(!pskb_may_pull(skb, sizeof(*hdr))))
   186			goto err;
   187	
   188		hdr = ipv6_hdr(skb);
   189	
   190		if (hdr->version != 6)
   191			goto err;
   192	
   193		__IP6_ADD_STATS(net, idev,
   194				IPSTATS_MIB_NOECTPKTS +
   195					(ipv6_get_dsfield(hdr) & INET_ECN_MASK),
   196				max_t(unsigned short, 1, skb_shinfo(skb)->gso_segs));
   197		/*
   198		 * RFC4291 2.5.3
   199		 * The loopback address must not be used as the source address in IPv6
   200		 * packets that are sent outside of a single node. [..]
   201		 * A packet received on an interface with a destination address
   202		 * of loopback must be dropped.
   203		 */
   204		if ((ipv6_addr_loopback(&hdr->saddr) ||
   205		     ipv6_addr_loopback(&hdr->daddr)) &&
   206		    !(dev->flags & IFF_LOOPBACK) &&
   207		    !netif_is_l3_master(dev))
   208			goto err;
   209	
   210		/* RFC4291 Errata ID: 3480
   211		 * Interface-Local scope spans only a single interface on a
   212		 * node and is useful only for loopback transmission of
   213		 * multicast.  Packets with interface-local scope received
   214		 * from another node must be discarded.
   215		 */
   216		if (!(skb->pkt_type == PACKET_LOOPBACK ||
   217		      dev->flags & IFF_LOOPBACK) &&
   218		    ipv6_addr_is_multicast(&hdr->daddr) &&
   219		    IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 1)
   220			goto err;
   221	
   222		/* If enabled, drop unicast packets that were encapsulated in link-layer
   223		 * multicast or broadcast to protected against the so-called "hole-196"
   224		 * attack in 802.11 wireless.
   225		 */
   226		if (!ipv6_addr_is_multicast(&hdr->daddr) &&
   227		    (skb->pkt_type == PACKET_BROADCAST ||
   228		     skb->pkt_type == PACKET_MULTICAST) &&
   229		    idev->cnf.drop_unicast_in_l2_multicast)
   230			goto err;
   231	
   232		/* RFC4291 2.7
   233		 * Nodes must not originate a packet to a multicast address whose scope
   234		 * field contains the reserved value 0; if such a packet is received, it
   235		 * must be silently dropped.
   236		 */
   237		if (ipv6_addr_is_multicast(&hdr->daddr) &&
   238		    IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 0)
   239			goto err;
   240	
   241		/*
   242		 * RFC4291 2.7
   243		 * Multicast addresses must not be used as source addresses in IPv6
   244		 * packets or appear in any Routing header.
   245		 */
   246		if (ipv6_addr_is_multicast(&hdr->saddr))
   247			goto err;
   248	
   249		skb->transport_header = skb->network_header + sizeof(*hdr);
   250		IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
   251	
   252		pkt_len = ntohs(hdr->payload_len);
   253	
   254		/* pkt_len may be zero if Jumbo payload option is present */
   255		if (pkt_len || hdr->nexthdr != NEXTHDR_HOP) {
   256			if (pkt_len + sizeof(struct ipv6hdr) > skb->len) {
   257				__IP6_INC_STATS(net,
   258						idev, IPSTATS_MIB_INTRUNCATEDPKTS);
   259				goto drop;
   260			}
   261			if (pskb_trim_rcsum(skb, pkt_len + sizeof(struct ipv6hdr))) {
   262				__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   263				goto drop;
   264			}
   265			hdr = ipv6_hdr(skb);
   266		}
   267	
   268		if (hdr->nexthdr == NEXTHDR_HOP) {
   269			if (ipv6_parse_hopopts(skb) < 0) {
   270				__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   271				rcu_read_unlock();
   272				return NULL;
   273			}
   274		}
   275	
   276		rcu_read_unlock();
   277	
   278		/* Must drop socket now because of tproxy. */
   279		if (!skb_sk_is_prefetched(skb))
   280			skb_orphan(skb);
   281	
   282		return skb;
   283	err:
   284		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
   285	drop:
   286		rcu_read_unlock();
   287		kfree_skb(skb);
   288		return NULL;
   289	}
   290	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-01-25  0:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-22  0:03 [PATCH net-next] net-core: add InMacErrors counter Jeffrey Ji
2022-01-22  3:40 ` Jakub Kicinski
2022-01-24 17:13   ` Brian Vazquez
2022-01-24 17:29     ` Jakub Kicinski
2022-01-24 17:39       ` Eric Dumazet
2022-01-24 17:46         ` Jakub Kicinski
2022-01-24 20:19       ` Cong Wang
2022-01-24 20:21         ` Eric Dumazet
2022-01-24 22:24 ` kernel test robot
2022-01-24 22:24 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).