2022-03-03 01:01:10

by Jeffrey Ji

[permalink] [raw]
Subject: [PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

From: jeffreyji <[email protected]>

Increment rx_otherhost_dropped counter when packet dropped due to
mismatched dest MAC addr.

An example when this drop can occur is when manually crafting raw
packets that will be consumed by a user space application via a tap
device. For testing purposes local traffic was generated using trafgen
for the client and netcat to start a server

Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
with "{eth(daddr=$INCORRECT_MAC...}", verified that iproute2 showed the
counter was incremented. (Also had to modify iproute2 to show the stat,
additional patch for that coming next.)

changelog:

v2: add kdoc comment

Signed-off-by: jeffreyji <[email protected]>
---
include/linux/netdevice.h | 3 +++
include/uapi/linux/if_link.h | 5 +++++
net/core/dev.c | 2 ++
net/ipv4/ip_input.c | 1 +
net/ipv6/ip6_input.c | 1 +
5 files changed, 12 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c79ee2296296..e4073c38bd77 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1741,6 +1741,8 @@ enum netdev_ml_priv_type {
* do not use this in drivers
* @rx_nohandler: nohandler dropped packets by core network on
* inactive devices, do not use this in drivers
+ * @rx_otherhost_dropped: Dropped packets due to mismatch in packet dest
+ * MAC address
* @carrier_up_count: Number of times the carrier has been up
* @carrier_down_count: Number of times the carrier has been down
*
@@ -2025,6 +2027,7 @@ struct net_device {
atomic_long_t rx_dropped;
atomic_long_t tx_dropped;
atomic_long_t rx_nohandler;
+ atomic_long_t rx_otherhost_dropped;

/* Stats to monitor link on/off, flapping */
atomic_t carrier_up_count;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index e315e53125f4..17e74385fca8 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -211,6 +211,9 @@ struct rtnl_link_stats {
* @rx_nohandler: Number of packets received on the interface
* but dropped by the networking stack because the device is
* not designated to receive packets (e.g. backup link in a bond).
+ *
+ * @rx_otherhost_dropped: Number of packets dropped due to mismatch in
+ * packet's destination MAC address.
*/
struct rtnl_link_stats64 {
__u64 rx_packets;
@@ -243,6 +246,8 @@ struct rtnl_link_stats64 {
__u64 rx_compressed;
__u64 tx_compressed;
__u64 rx_nohandler;
+
+ __u64 rx_otherhost_dropped;
};

/* The struct should be in sync with struct ifmap */
diff --git a/net/core/dev.c b/net/core/dev.c
index 2d6771075720..d039d8fdc16a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10037,6 +10037,8 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
storage->rx_dropped += (unsigned long)atomic_long_read(&dev->rx_dropped);
storage->tx_dropped += (unsigned long)atomic_long_read(&dev->tx_dropped);
storage->rx_nohandler += (unsigned long)atomic_long_read(&dev->rx_nohandler);
+ storage->rx_otherhost_dropped +=
+ (unsigned long)atomic_long_read(&dev->rx_otherhost_dropped);
return storage;
}
EXPORT_SYMBOL(dev_get_stats);
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index d94f9f7e60c3..ef97b0a4c77f 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -450,6 +450,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
* that it receives, do not try to analyse it.
*/
if (skb->pkt_type == PACKET_OTHERHOST) {
+ atomic_long_inc(&skb->dev->rx_otherhost_dropped);
drop_reason = SKB_DROP_REASON_OTHERHOST;
goto drop;
}
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index d4b1e2c5aa76..3f0cbe126d82 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) {
+ atomic_long_inc(&skb->dev->rx_otherhost_dropped);
kfree_skb(skb);
return NULL;
}
--
2.35.1.616.g0bdcbb4464-goog


2022-03-03 22:29:01

by Brian Vazquez

[permalink] [raw]
Subject: Re: [PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

LGTM, thanks Jeffrey!

Reviewed-by: Brian Vazquez <[email protected]>

On Wed, Mar 2, 2022 at 4:30 PM Jeffrey Ji <[email protected]> wrote:
>
> From: jeffreyji <[email protected]>
>
> Increment rx_otherhost_dropped counter when packet dropped due to
> mismatched dest MAC addr.
>
> An example when this drop can occur is when manually crafting raw
> packets that will be consumed by a user space application via a tap
> device. For testing purposes local traffic was generated using trafgen
> for the client and netcat to start a server
>
> Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> with "{eth(daddr=$INCORRECT_MAC...}", verified that iproute2 showed the
> counter was incremented. (Also had to modify iproute2 to show the stat,
> additional patch for that coming next.)
>
> changelog:
>
> v2: add kdoc comment
>
> Signed-off-by: jeffreyji <[email protected]>
> ---
> include/linux/netdevice.h | 3 +++
> include/uapi/linux/if_link.h | 5 +++++
> net/core/dev.c | 2 ++
> net/ipv4/ip_input.c | 1 +
> net/ipv6/ip6_input.c | 1 +
> 5 files changed, 12 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c79ee2296296..e4073c38bd77 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1741,6 +1741,8 @@ enum netdev_ml_priv_type {
> * do not use this in drivers
> * @rx_nohandler: nohandler dropped packets by core network on
> * inactive devices, do not use this in drivers
> + * @rx_otherhost_dropped: Dropped packets due to mismatch in packet dest
> + * MAC address
> * @carrier_up_count: Number of times the carrier has been up
> * @carrier_down_count: Number of times the carrier has been down
> *
> @@ -2025,6 +2027,7 @@ struct net_device {
> atomic_long_t rx_dropped;
> atomic_long_t tx_dropped;
> atomic_long_t rx_nohandler;
> + atomic_long_t rx_otherhost_dropped;
>
> /* Stats to monitor link on/off, flapping */
> atomic_t carrier_up_count;
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index e315e53125f4..17e74385fca8 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -211,6 +211,9 @@ struct rtnl_link_stats {
> * @rx_nohandler: Number of packets received on the interface
> * but dropped by the networking stack because the device is
> * not designated to receive packets (e.g. backup link in a bond).
> + *
> + * @rx_otherhost_dropped: Number of packets dropped due to mismatch in
> + * packet's destination MAC address.
> */
> struct rtnl_link_stats64 {
> __u64 rx_packets;
> @@ -243,6 +246,8 @@ struct rtnl_link_stats64 {
> __u64 rx_compressed;
> __u64 tx_compressed;
> __u64 rx_nohandler;
> +
> + __u64 rx_otherhost_dropped;
> };
>
> /* The struct should be in sync with struct ifmap */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2d6771075720..d039d8fdc16a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10037,6 +10037,8 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
> storage->rx_dropped += (unsigned long)atomic_long_read(&dev->rx_dropped);
> storage->tx_dropped += (unsigned long)atomic_long_read(&dev->tx_dropped);
> storage->rx_nohandler += (unsigned long)atomic_long_read(&dev->rx_nohandler);
> + storage->rx_otherhost_dropped +=
> + (unsigned long)atomic_long_read(&dev->rx_otherhost_dropped);
> return storage;
> }
> EXPORT_SYMBOL(dev_get_stats);
> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index d94f9f7e60c3..ef97b0a4c77f 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -450,6 +450,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
> * that it receives, do not try to analyse it.
> */
> if (skb->pkt_type == PACKET_OTHERHOST) {
> + atomic_long_inc(&skb->dev->rx_otherhost_dropped);
> drop_reason = SKB_DROP_REASON_OTHERHOST;
> goto drop;
> }
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index d4b1e2c5aa76..3f0cbe126d82 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) {
> + atomic_long_inc(&skb->dev->rx_otherhost_dropped);
> kfree_skb(skb);
> return NULL;
> }
> --
> 2.35.1.616.g0bdcbb4464-goog
>

2022-03-05 06:22:54

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

On Thu, 3 Mar 2022 13:59:58 -0800 Brian Vazquez wrote:
> LGTM, thanks Jeffrey!
>
> Reviewed-by: Brian Vazquez <[email protected]>

Please keep Brian's review tag..

> On Wed, Mar 2, 2022 at 4:30 PM Jeffrey Ji <[email protected]> wrote:
> >
> > From: jeffreyji <[email protected]>
> >
> > Increment rx_otherhost_dropped counter when packet dropped due to
> > mismatched dest MAC addr.
> >
> > An example when this drop can occur is when manually crafting raw
> > packets that will be consumed by a user space application via a tap
> > device. For testing purposes local traffic was generated using trafgen
> > for the client and netcat to start a server
> >
> > Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> > with "{eth(daddr=$INCORRECT_MAC...}", verified that iproute2 showed the
> > counter was incremented. (Also had to modify iproute2 to show the stat,
> > additional patch for that coming next.)
> >
> > changelog:
> >
> > v2: add kdoc comment

move the changelog under the '---' separator

> > Signed-off-by: jeffreyji <[email protected]>
> > ---
> > include/linux/netdevice.h | 3 +++
> > include/uapi/linux/if_link.h | 5 +++++
> > net/core/dev.c | 2 ++
> > net/ipv4/ip_input.c | 1 +
> > net/ipv6/ip6_input.c | 1 +
> > 5 files changed, 12 insertions(+)

> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index e315e53125f4..17e74385fca8 100644
> > --- a/include/uapi/linux/if_link.h
> > +++ b/include/uapi/linux/if_link.h
> > @@ -211,6 +211,9 @@ struct rtnl_link_stats {
> > * @rx_nohandler: Number of packets received on the interface
> > * but dropped by the networking stack because the device is
> > * not designated to receive packets (e.g. backup link in a bond).
> > + *
> > + * @rx_otherhost_dropped: Number of packets dropped due to mismatch in
> > + * packet's destination MAC address.
> > */
> > struct rtnl_link_stats64 {
> > __u64 rx_packets;
> > @@ -243,6 +246,8 @@ struct rtnl_link_stats64 {
> > __u64 rx_compressed;
> > __u64 tx_compressed;
> > __u64 rx_nohandler;
> > +
> > + __u64 rx_otherhost_dropped;
> > };
> >
> > /* The struct should be in sync with struct ifmap */

rebase on latest net-next/master and repost.

Looks like it's conflicting with recently merged
commit 9309f97aef6d ("net: dev: Add hardware stats support")