Skip to contentSkip to conversation

be2net: workaround to fix a BE bug

PATCH In review Ajit Khaparde 3 participants 4 messages Dec 7, 2011 – Dec 8, 2011

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

PATCH 2/2 be2net: workaround to fix a BE bug

For vlan tagged pkts, BE
1) calculates checksum even when CSO is not requested
2) calculates checksum wrongly for padded pkt less than 60 bytes long. As a workaround disable TX vlan offloading in such cases.

---
 drivers/net/ethernet/emulex/benet/be.h      |   16 ++++++++++++++
 drivers/net/ethernet/emulex/benet/be_main.c |   30 +++++++++++++++++++-------
 2 files changed, 38 insertions(+), 8 deletions(-)
drivers/net/ethernet/emulex/benet/be.h +16 -0
index 34f162d..c4fcea69 100644
@@ -518,6 +518,22 @@ static inline void be_vf_eth_addr_generate(struct be_adapter *adapter, u8 *mac)
 	memcpy(mac, adapter->netdev->dev_addr, 3);
 }
 
+static inline u16 be_get_tx_vlan_tag(struct be_adapter *adapter,
+					struct sk_buff *skb)
+{
+	u8 vlan_prio;
+	u16 vlan_tag;
+
+	vlan_tag = vlan_tx_tag_get(skb);
+	vlan_prio = (vlan_tag & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+	/* If vlan priority provided by OS is NOT in available bmap */
+	if (!(adapter->vlan_prio_bmap & (1 << vlan_prio)))
+		vlan_tag = (vlan_tag & ~VLAN_PRIO_MASK) |
+				adapter->recommended_prio;
+
+	return vlan_tag;
+}
+
 static inline bool be_multi_rxq(const struct be_adapter *adapter)
 {
 	return adapter->num_rx_qs > 1;
drivers/net/ethernet/emulex/benet/be_main.c +22 -8
index b29c447..c5c826f 100644
@@ -555,8 +555,7 @@ static inline void wrb_fill(struct be_eth_wrb *wrb, u64 addr, int len)
 static void wrb_fill_hdr(struct be_adapter *adapter, struct be_eth_hdr_wrb *hdr,
 		struct sk_buff *skb, u32 wrb_cnt, u32 len)
 {
-	u8 vlan_prio = 0;
-	u16 vlan_tag = 0;
+	u16 vlan_tag;
 
 	memset(hdr, 0, sizeof(*hdr));
 
@@ -587,12 +586,7 @@ static void wrb_fill_hdr(struct be_adapter *adapter, struct be_eth_hdr_wrb *hdr,
 
 	if (vlan_tx_tag_present(skb)) {
 		AMAP_SET_BITS(struct amap_eth_hdr_wrb, vlan, hdr, 1);
-		vlan_tag = vlan_tx_tag_get(skb);
-		vlan_prio = (vlan_tag & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
-		/* If vlan priority provided by OS is NOT in available bmap */
-		if (!(adapter->vlan_prio_bmap & (1 << vlan_prio)))
-			vlan_tag = (vlan_tag & ~VLAN_PRIO_MASK) |
-					adapter->recommended_prio;
+		vlan_tag = be_get_tx_vlan_tag(adapter, skb);
 		AMAP_SET_BITS(struct amap_eth_hdr_wrb, vlan_tag, hdr, vlan_tag);
 	}
 
@@ -695,6 +689,25 @@ static netdev_tx_t be_xmit(struct sk_buff *skb,
 	u32 start = txq->head;
 	bool dummy_wrb, stopped = false;
 
+	/* For vlan tagged pkts, BE
+	 * 1) calculates checksum even when CSO is not requested
+	 * 2) calculates checksum wrongly for padded pkt less than
+	 * 60 bytes long.
+	 * As a workaround disable TX vlan offloading in such cases.
+	 */
+	if (unlikely(vlan_tx_tag_present(skb) &&
+		(skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {
+		skb = skb_share_check(skb, GFP_ATOMIC);
+		if (unlikely(!skb))
+			goto tx_drop;
+
+		skb = __vlan_put_tag(skb, be_get_tx_vlan_tag(adapter, skb));
+		if (unlikely(!skb))
+			goto tx_drop;
+
+		skb->vlan_tci = 0;
+	}
+
 	wrb_cnt = wrb_cnt_for_skb(adapter, skb, &dummy_wrb);
 
 	copied = make_tx_wrbs(adapter, txq, skb, wrb_cnt, dummy_wrb);
@@ -722,6 +735,7 @@ static netdev_tx_t be_xmit(struct sk_buff *skb,
 		txq->head = start;
 		dev_kfree_skb_any(skb);
 	}
+tx_drop:
 	return NETDEV_TX_OK;
 }
 
Signature · 1 line
1.7.5.4
maintainer
#1 lore raw

From: Ajit Khaparde <[email protected]>
Date: Wed, 7 Dec 2011 17:44:24 -0600

Quoted 4 lines

For vlan tagged pkts, BE
1) calculates checksum even when CSO is not requested
2) calculates checksum wrongly for padded pkt less than 60 bytes long. As a workaround disable TX vlan offloading in such cases.

...

drivers/net/ethernet/emulex/benet/be_main.c static netdev_tx_t be_xmit(struct sk_buff *skb, · line 697

Quoted 6 lines
drivers/net/ethernet/emulex/benet/be_main.c +6 -0
+	/* For vlan tagged pkts, BE
+	 * 1) calculates checksum even when CSO is not requested
+	 * 2) calculates checksum wrongly for padded pkt less than
+	 * 60 bytes long.
+	 * As a workaround disable TX vlan offloading in such cases.
+	 */

This description either belongs in the commit message, or this comment, but not both.

drivers/net/ethernet/emulex/benet/be_main.c static netdev_tx_t be_xmit(struct sk_buff *skb, · line 699

Quoted 2 lines
drivers/net/ethernet/emulex/benet/be_main.c +2 -0
+	if (unlikely(vlan_tx_tag_present(skb) &&
+		(skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {

Poorly formatted, the second line openning first non-space character needs to line up precisely after the first column after the if() statements openning parenthesis.

If you cannot do this yourself by hand, use something like emacs's C-mode to do it automatically for you when you hit TAB on that second line.

#2 lore raw
Quoted 6 lines · David Miller

From: Ajit Khaparde <[email protected]>

drivers/net/ethernet/emulex/benet/be_main.c static netdev_tx_t be_xmit(struct sk_buff *skb, · line 699

Quoted 2 lines
drivers/net/ethernet/emulex/benet/be_main.c +2 -0
+	if (unlikely(vlan_tx_tag_present(skb) &&
+		(skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {

Poorly formatted, the second line opening first non-space character needs to line up precisely after the first column after the if() statements opening parenthesis.

In this case I believe you mean after the unlikely(

	if (unlikely(vlan_tx_tag_present(skb) &&
		     (skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {
maintainer
#3 lore raw

From: Joe Perches <[email protected]>
Date: Wed, 07 Dec 2011 21:07:20 -0800

Quoted 8 lines · David Miller
Quoted 6 lines

From: Ajit Khaparde <[email protected]>

drivers/net/ethernet/emulex/benet/be_main.c static netdev_tx_t be_xmit(struct sk_buff *skb, · line 699

Quoted 2 lines
drivers/net/ethernet/emulex/benet/be_main.c +2 -0
+	if (unlikely(vlan_tx_tag_present(skb) &&
+		(skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {

Poorly formatted, the second line opening first non-space character needs to line up precisely after the first column after the if() statements opening parenthesis.

Quoted 4 lines

In this case I believe you mean after the unlikely(

	if (unlikely(vlan_tx_tag_present(skb) &&
		     (skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {

Right.