| Message ID | 20111207234424.GA2385@akhaparde-VBox |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Ajit Khaparde <ajit.khaparde@Emulex.Com> Date: Wed, 7 Dec 2011 17:44:24 -0600 > 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. ... > + /* 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. > + 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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2011-12-08 at 00:00 -0500, David Miller wrote: > From: Ajit Khaparde <ajit.khaparde@Emulex.Com> > > + 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))) { -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Wed, 07 Dec 2011 21:07:20 -0800 > On Thu, 2011-12-08 at 00:00 -0500, David Miller wrote: >> From: Ajit Khaparde <ajit.khaparde@Emulex.Com> >> > + 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))) { Right. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h index 34f162d..c4fcea69 100644 --- a/drivers/net/ethernet/emulex/benet/be.h +++ b/drivers/net/ethernet/emulex/benet/be.h @@ -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; diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c index b29c447..c5c826f 100644 --- a/drivers/net/ethernet/emulex/benet/be_main.c +++ b/drivers/net/ethernet/emulex/benet/be_main.c @@ -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; }
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. Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com> --- drivers/net/ethernet/emulex/benet/be.h | 16 ++++++++++++++ drivers/net/ethernet/emulex/benet/be_main.c | 30 +++++++++++++++++++------- 2 files changed, 38 insertions(+), 8 deletions(-)