Skip to contentSkip to conversation

net: qualcomm: rmnet: Updates 2018-05-14

PATCH In review Subash Abhinov Kasiviswanathan 2 participants 5 messages May 16, 2018

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover net: qualcomm: rmnet: Updates 2018-05-14

Patch 1 adds tx_drops counter to more places.
Patch 2 adds ethtool private stats support to make it easy to debug the checksum offload path.
Patch 3 is a cleanup in command packet processing path.

v1->v2: Fix the incorrect if / else statement in rmnet_map_checksum_downlink_packet() and define rmnet_ethtool_ops as static as mentioned by kbuild test robot.

Subash Abhinov Kasiviswanathan (3):
net: qualcomm: rmnet: Capture all drops in transmit path
net: qualcomm: rmnet: Add support for ethtool private stats
net: qualcomm: rmnet: Remove redundant command check

 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 13 +++++
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 21 ++++---
 .../ethernet/qualcomm/rmnet/rmnet_map_command.c    | 14 +----
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 64 ++++++++++++++++------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    | 51 +++++++++++++++++
 5 files changed, 125 insertions(+), 38 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH v2 1/3 net: qualcomm: rmnet: Capture all drops in transmit path

Packets in transmit path could potentially be dropped if there were errors while adding the MAP header or the checksum header. Increment the tx_drops stats in these cases.

Additionally, refactor the code to free the packet and increment the tx_drops stat under a single label.

---
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c    | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c +10 -11
index 6fcd586..7fd86d4 100644
@@ -148,7 +148,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 
 	if (skb_headroom(skb) < required_headroom) {
 		if (pskb_expand_head(skb, required_headroom, 0, GFP_KERNEL))
-			goto fail;
+			return -ENOMEM;
 	}
 
 	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4)
@@ -156,17 +156,13 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 
 	map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
 	if (!map_header)
-		goto fail;
+		return -ENOMEM;
 
 	map_header->mux_id = mux_id;
 
 	skb->protocol = htons(ETH_P_MAP);
 
 	return 0;
-
-fail:
-	kfree_skb(skb);
-	return -ENOMEM;
 }
 
 static void
@@ -228,15 +224,18 @@ void rmnet_egress_handler(struct sk_buff *skb)
 	mux_id = priv->mux_id;
 
 	port = rmnet_get_port(skb->dev);
-	if (!port) {
-		kfree_skb(skb);
-		return;
-	}
+	if (!port)
+		goto drop;
 
 	if (rmnet_map_egress_handler(skb, port, mux_id, orig_dev))
-		return;
+		goto drop;
 
 	rmnet_vnd_tx_fixup(skb, orig_dev);
 
 	dev_queue_xmit(skb);
+	return;
+
+drop:
+	this_cpu_inc(priv->pcpu_stats->stats.tx_drops);
+	kfree_skb(skb);
 }
Signature · 1 line
1.9.1
#2 lore raw

PATCH v2 2/3 net: qualcomm: rmnet: Add support for ethtool private stats

In reply to Subash Abhinov Kasiviswanathan

Add ethtool private stats handler to debug the handling of packets with checksum offload header / trailer. This allows to keep track of the number of packets for which hardware computes the checksum and counts and reasons where checksum computation was skipped in hardware and was done in the network stack.

---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 13 +++++
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 64 ++++++++++++++++------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    | 51 +++++++++++++++++
 3 files changed, 112 insertions(+), 16 deletions(-)
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h +13 -0
index 0b5b5da..34ac45a 100644
@@ -54,11 +54,24 @@ struct rmnet_pcpu_stats {
 	struct u64_stats_sync syncp;
 };
 
+struct rmnet_priv_stats {
+	u64 csum_ok;
+	u64 csum_valid_unset;
+	u64 csum_validation_failed;
+	u64 csum_err_bad_buffer;
+	u64 csum_err_invalid_ip_version;
+	u64 csum_err_invalid_transport;
+	u64 csum_fragmented_pkt;
+	u64 csum_skipped;
+	u64 csum_sw;
+};
+
 struct rmnet_priv {
 	u8 mux_id;
 	struct net_device *real_dev;
 	struct rmnet_pcpu_stats __percpu *pcpu_stats;
 	struct gro_cells gro_cells;
+	struct rmnet_priv_stats stats;
 };
 
 struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c +48 -16

150 lines not on this page · raw message

drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c +51 -0

63 lines not on this page · raw message

Signature · 1 line
1.9.1
#3 lore raw

PATCH v2 3/3 net: qualcomm: rmnet: Remove redundant command check

In reply to Subash Abhinov Kasiviswanathan

The command packet size is already checked once in rmnet_map_deaggregate() for the header, packet and trailer size, so this additional check is not needed.

---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c +3 -11
index 78fdad0..56a93df 100644
@@ -69,17 +69,9 @@ static void rmnet_map_send_ack(struct sk_buff *skb,
 	struct rmnet_map_control_command *cmd;
 	int xmit_status;
 
-	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
-		if (skb->len < sizeof(struct rmnet_map_header) +
-		    RMNET_MAP_GET_LENGTH(skb) +
-		    sizeof(struct rmnet_map_dl_csum_trailer)) {
-			kfree_skb(skb);
-			return;
-		}
-
-		skb_trim(skb, skb->len -
-			 sizeof(struct rmnet_map_dl_csum_trailer));
-	}
+	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
+		skb_trim(skb,
+			 skb->len - sizeof(struct rmnet_map_dl_csum_trailer));
 
 	skb->protocol = htons(ETH_P_MAP);
 
Signature · 1 line
1.9.1
maintainer
#4 lore raw
In reply to Subash Abhinov Kasiviswanathan

From: Subash Abhinov Kasiviswanathan <[email protected]>
Date: Tue, 15 May 2018 18:51:59 -0600

Quoted 8 lines

Patch 1 adds tx_drops counter to more places.
Patch 2 adds ethtool private stats support to make it easy to debug the checksum offload path.
Patch 3 is a cleanup in command packet processing path.

v1->v2: Fix the incorrect if / else statement in rmnet_map_checksum_downlink_packet() and define rmnet_ethtool_ops as static as mentioned by kbuild test robot.

Series applied, thank you.