lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Thu, 29 Nov 2018 20:00:07 -0500
From:   Debabrata Banerjee <dbanerje@...mai.com>
To:     "David S . Miller" <davem@...emloft.net>,
        Alexander Duyck <alexander.h.duyck@...el.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        netdev@...r.kernel.org
Cc:     dbanerje@...mai.com
Subject: [PATCH RFC net-next v2] net: vlan/macvlan: count packets properly with gso

Fix packet count when using vlan/macvlan drivers with gso. Without this it
is not possible to reconcile packet counts between underlying devices and
these virtual devices. Additionally, the output looks wrong in a standalone
way i.e. device MTU of 1500, 1 packet sent, 31856 bytes sent.

There are many other drivers that likely have a similar problem, although
it is not clear how many of those could be used with gso. Perhaps all
packet counting should be wrapped in a helper fn.

v2: bytes were also incorrect for gso skb's, fix tx as that is readily
available. Fix rx packets for macvlan.

Signed-off-by: Debabrata Banerjee <dbanerje@...mai.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |  4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  4 ++--
 drivers/net/macvlan.c                         | 13 ++++++++-----
 drivers/net/macvtap.c                         |  2 +-
 include/linux/if_macvlan.h                    |  6 +++---
 net/8021q/vlan_core.c                         |  3 ++-
 net/8021q/vlan_dev.c                          |  4 ++--
 7 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 6fd15a734324..e39fad2d888f 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -431,8 +431,8 @@ static void fm10k_type_trans(struct fm10k_ring *rx_ring,
 	if (!l2_accel)
 		skb_record_rx_queue(skb, rx_ring->queue_index);
 	else
-		macvlan_count_rx(netdev_priv(dev), skb->len + ETH_HLEN, true,
-				 false);
+		macvlan_count_rx(netdev_priv(dev), skb_shinfo(skb)->gso_segs ?: 1,
+				 skb->len + ETH_HLEN, true, false);
 
 	skb->protocol = eth_type_trans(skb, dev);
 }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 49a4ea38eb07..474e72ec68b0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1704,8 +1704,8 @@ void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
 	if (netif_is_ixgbe(dev))
 		skb_record_rx_queue(skb, rx_ring->queue_index);
 	else
-		macvlan_count_rx(netdev_priv(dev), skb->len + ETH_HLEN, true,
-				 false);
+		macvlan_count_rx(netdev_priv(dev), skb_shinfo(skb)->gso_segs ?: 1,
+				 skb->len + ETH_HLEN, true, false);
 
 	skb->protocol = eth_type_trans(skb, dev);
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index fc8d5f1ee1ad..ab8743777897 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -289,7 +289,8 @@ static void macvlan_broadcast(struct sk_buff *skb,
 					nskb, vlan, eth,
 					mode == MACVLAN_MODE_BRIDGE) ?:
 				      netif_rx_ni(nskb);
-			macvlan_count_rx(vlan, skb->len + ETH_HLEN,
+			macvlan_count_rx(vlan, skb_shinfo(skb)->gso_segs ?: 1,
+					 skb->len + ETH_HLEN,
 					 err == NET_RX_SUCCESS, true);
 		}
 	}
@@ -418,7 +419,8 @@ static void macvlan_forward_source_one(struct sk_buff *skb,
 		nskb->pkt_type = PACKET_HOST;
 
 	ret = netif_rx(nskb);
-	macvlan_count_rx(vlan, len, ret == NET_RX_SUCCESS, false);
+	macvlan_count_rx(vlan, skb_shinfo(skb)->gso_segs ?: 1, len,
+			 ret == NET_RX_SUCCESS, false);
 }
 
 static void macvlan_forward_source(struct sk_buff *skb,
@@ -505,7 +507,8 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 	ret = NET_RX_SUCCESS;
 	handle_res = RX_HANDLER_ANOTHER;
 out:
-	macvlan_count_rx(vlan, len, ret == NET_RX_SUCCESS, false);
+	macvlan_count_rx(vlan, skb_shinfo(skb)->gso_segs ?: 1, len,
+			 ret == NET_RX_SUCCESS, false);
 	return handle_res;
 }
 
@@ -553,7 +556,7 @@ static netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
-	unsigned int len = skb->len;
+	unsigned int len = qdisc_skb_cb(skb)->pkt_len;
 	int ret;
 
 	if (unlikely(netpoll_tx_running(dev)))
@@ -566,7 +569,7 @@ static netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
 
 		pcpu_stats = this_cpu_ptr(vlan->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->tx_packets++;
+		pcpu_stats->tx_packets += skb_shinfo(skb)->gso_segs ?: 1;
 		pcpu_stats->tx_bytes += len;
 		u64_stats_update_end(&pcpu_stats->syncp);
 	} else {
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 9a10029caf83..06880a61cab9 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -64,7 +64,7 @@ static void macvtap_count_rx_dropped(struct tap_dev *tap)
 	struct macvtap_dev *vlantap = container_of(tap, struct macvtap_dev, tap);
 	struct macvlan_dev *vlan = &vlantap->vlan;
 
-	macvlan_count_rx(vlan, 0, 0, 0);
+	macvlan_count_rx(vlan, 1, 0, 0, 0);
 }
 
 static void macvtap_update_features(struct tap_dev *tap,
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index 2e55e4cdbd8a..0f592925c559 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -37,15 +37,15 @@ struct macvlan_dev {
 };
 
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
-				    unsigned int len, bool success,
-				    bool multicast)
+				    unsigned int packets, unsigned int len,
+				    bool success, bool multicast)
 {
 	if (likely(success)) {
 		struct vlan_pcpu_stats *pcpu_stats;
 
 		pcpu_stats = this_cpu_ptr(vlan->pcpu_stats);
 		u64_stats_update_begin(&pcpu_stats->syncp);
-		pcpu_stats->rx_packets++;
+		pcpu_stats->rx_packets += packets;
 		pcpu_stats->rx_bytes += len;
 		if (multicast)
 			pcpu_stats->rx_multicast++;
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index a313165e7a67..3297eac36541 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -4,6 +4,7 @@
 #include <linux/if_vlan.h>
 #include <linux/netpoll.h>
 #include <linux/export.h>
+#include <net/sch_generic.h>
 #include "vlan.h"
 
 bool vlan_do_receive(struct sk_buff **skbp)
@@ -62,7 +63,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
 	rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
 
 	u64_stats_update_begin(&rx_stats->syncp);
-	rx_stats->rx_packets++;
+	rx_stats->rx_packets += skb_shinfo(skb)->gso_segs ?: 1;
 	rx_stats->rx_bytes += skb->len;
 	if (skb->pkt_type == PACKET_MULTICAST)
 		rx_stats->rx_multicast++;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b2d9c8f27cd7..0e4aca7d594f 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -124,7 +124,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 	}
 
 	skb->dev = vlan->real_dev;
-	len = skb->len;
+	len = qdisc_skb_cb(skb)->pkt_len;
 	if (unlikely(netpoll_tx_running(dev)))
 		return vlan_netpoll_send_skb(vlan, skb);
 
@@ -135,7 +135,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 
 		stats = this_cpu_ptr(vlan->vlan_pcpu_stats);
 		u64_stats_update_begin(&stats->syncp);
-		stats->tx_packets++;
+		stats->tx_packets += skb_shinfo(skb)->gso_segs ?: 1;
 		stats->tx_bytes += len;
 		u64_stats_update_end(&stats->syncp);
 	} else {
-- 
2.19.2

Powered by blists - more mailing lists