Re: [PATCH net 3/3] net: bcmgenet: use hardware padding of runt frames

2020-06-24 Thread Florian Fainelli



On 6/24/2020 6:14 PM, Doug Berger wrote:
> When commit 474ea9cafc45 ("net: bcmgenet: correctly pad short
> packets") added the call to skb_padto() it should have been
> located before the nr_frags parameter was read since that value
> could be changed when padding packets with lengths between 55
> and 59 bytes (inclusive).
> 
> The use of a stale nr_frags value can cause corruption of the
> pad data when tx-scatter-gather is enabled. This corruption of
> the pad can cause invalid checksum computation when hardware
> offload of tx-checksum is also enabled.
> 
> Since the original reason for the padding was corrected by
> commit 7dd399130efb ("net: bcmgenet: fix skb_len in
> bcmgenet_xmit_single()") we can remove the software padding all
> together and make use of hardware padding of short frames as
> long as the hardware also always appends the FCS value to the
> frame.
> 
> Fixes: 474ea9cafc45 ("net: bcmgenet: correctly pad short packets")
> Signed-off-by: Doug Berger 

Acked-by: Florian Fainelli 
-- 
Florian


[PATCH net 3/3] net: bcmgenet: use hardware padding of runt frames

2020-06-24 Thread Doug Berger
When commit 474ea9cafc45 ("net: bcmgenet: correctly pad short
packets") added the call to skb_padto() it should have been
located before the nr_frags parameter was read since that value
could be changed when padding packets with lengths between 55
and 59 bytes (inclusive).

The use of a stale nr_frags value can cause corruption of the
pad data when tx-scatter-gather is enabled. This corruption of
the pad can cause invalid checksum computation when hardware
offload of tx-checksum is also enabled.

Since the original reason for the padding was corrected by
commit 7dd399130efb ("net: bcmgenet: fix skb_len in
bcmgenet_xmit_single()") we can remove the software padding all
together and make use of hardware padding of short frames as
long as the hardware also always appends the FCS value to the
frame.

Fixes: 474ea9cafc45 ("net: bcmgenet: correctly pad short packets")
Signed-off-by: Doug Berger 
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index c63f01e2bb03..af924a8b885f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2042,11 +2042,6 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, 
struct net_device *dev)
goto out;
}
 
-   if (skb_padto(skb, ETH_ZLEN)) {
-   ret = NETDEV_TX_OK;
-   goto out;
-   }
-
/* Retain how many bytes will be sent on the wire, without TSB inserted
 * by transmit checksum offload
 */
@@ -2093,6 +2088,9 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, 
struct net_device *dev)
len_stat = (size << DMA_BUFLENGTH_SHIFT) |
   (priv->hw_params->qtag_mask << DMA_TX_QTAG_SHIFT);
 
+   /* Note: if we ever change from DMA_TX_APPEND_CRC below we
+* will need to restore software padding of "runt" packets
+*/
if (!i) {
len_stat |= DMA_TX_APPEND_CRC | DMA_SOP;
if (skb->ip_summed == CHECKSUM_PARTIAL)
-- 
2.7.4