diff mbox series

[net-next,1/3] net: bgmac: Pad packets to a minimum size

Message ID 20171109222606.2987-2-f.fainelli@gmail.com
State Superseded, archived
Delegated to: David Miller
Headers show
Series net: dsa: b53: Turn on Broadcom tags | expand

Commit Message

Florian Fainelli Nov. 9, 2017, 10:26 p.m. UTC
In preparation for enabling Broadcom tags with b53, pad packets to a
minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
accept ingressing frames. Without this, we would typically be able to
DHCP, but not resolve with ARP because packets are too small and get
rejected by the switch.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Andrew Lunn Nov. 9, 2017, 10:37 p.m. UTC | #1
On Thu, Nov 09, 2017 at 02:26:04PM -0800, Florian Fainelli wrote:
> In preparation for enabling Broadcom tags with b53, pad packets to a
> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
> accept ingressing frames. Without this, we would typically be able to
> DHCP, but not resolve with ARP because packets are too small and get
> rejected by the switch.

Hi Florian

Is the MAC sending runt packets in its default configuration? Is this
a general issue, and not just an issue when there is a switch directly
attached?

Thanks
	Andrew
Florian Fainelli Nov. 9, 2017, 11:03 p.m. UTC | #2
On 11/09/2017 02:37 PM, Andrew Lunn wrote:
> On Thu, Nov 09, 2017 at 02:26:04PM -0800, Florian Fainelli wrote:
>> In preparation for enabling Broadcom tags with b53, pad packets to a
>> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
>> accept ingressing frames. Without this, we would typically be able to
>> DHCP, but not resolve with ARP because packets are too small and get
>> rejected by the switch.
> 
> Hi Florian
> 
> Is the MAC sending runt packets in its default configuration? Is this
> a general issue, and not just an issue when there is a switch directly
> attached?

The MAC is sending 64 bytes (with FCS) padded packets by default, but
this apparently gets mis-calculated when Broadcom tags are enabled, such
that we need to pad before to avoid that.
Andrew Lunn Nov. 10, 2017, 1:06 a.m. UTC | #3
On Thu, Nov 09, 2017 at 03:03:16PM -0800, Florian Fainelli wrote:
> On 11/09/2017 02:37 PM, Andrew Lunn wrote:
> > On Thu, Nov 09, 2017 at 02:26:04PM -0800, Florian Fainelli wrote:
> >> In preparation for enabling Broadcom tags with b53, pad packets to a
> >> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
> >> accept ingressing frames. Without this, we would typically be able to
> >> DHCP, but not resolve with ARP because packets are too small and get
> >> rejected by the switch.
> > 
> > Hi Florian
> > 
> > Is the MAC sending runt packets in its default configuration? Is this
> > a general issue, and not just an issue when there is a switch directly
> > attached?
> 
> The MAC is sending 64 bytes (with FCS) padded packets by default, but
> this apparently gets mis-calculated when Broadcom tags are enabled, such
> that we need to pad before to avoid that.

Hi Florian

Ah, so maybe when the tag is stripped off it then becomes a runt
packet and so gets dropped.

   Andrew
Florian Fainelli Nov. 10, 2017, 1:25 a.m. UTC | #4
On 11/09/2017 05:06 PM, Andrew Lunn wrote:
> On Thu, Nov 09, 2017 at 03:03:16PM -0800, Florian Fainelli wrote:
>> On 11/09/2017 02:37 PM, Andrew Lunn wrote:
>>> On Thu, Nov 09, 2017 at 02:26:04PM -0800, Florian Fainelli wrote:
>>>> In preparation for enabling Broadcom tags with b53, pad packets to a
>>>> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
>>>> accept ingressing frames. Without this, we would typically be able to
>>>> DHCP, but not resolve with ARP because packets are too small and get
>>>> rejected by the switch.
>>>
>>> Hi Florian
>>>
>>> Is the MAC sending runt packets in its default configuration? Is this
>>> a general issue, and not just an issue when there is a switch directly
>>> attached?
>>
>> The MAC is sending 64 bytes (with FCS) padded packets by default, but
>> this apparently gets mis-calculated when Broadcom tags are enabled, such
>> that we need to pad before to avoid that.
> 
> Hi Florian
> 
> Ah, so maybe when the tag is stripped off it then becomes a runt
> packet and so gets dropped.

Yes, that is exactly what I observed.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 48d672b204a4..5130fc96940d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -127,6 +127,8 @@  bgmac_dma_tx_add_buf(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	dma_desc->ctl1 = cpu_to_le32(ctl1);
 }
 
+#define ENET_BRCM_TAG_LEN	4
+
 static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 				    struct bgmac_dma_ring *ring,
 				    struct sk_buff *skb)
@@ -139,6 +141,16 @@  static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	u32 flags;
 	int i;
 
+	/* The Ethernet switch we are interfaced with needs packets to be at
+	 * least 64 bytes (including FCS) otherwise they will be discarded when
+	 * they enter the switch port logic. When Broadcom tags are enabled, we
+	 * need to make sure that packets are at least 68 bytes
+	 * (including FCS and tag) because the length verification is done after
+	 * the Broadcom tag is stripped off the ingress packet.
+	 */
+	if (skb_put_padto(skb, ETH_ZLEN + ENET_BRCM_TAG_LEN))
+		goto err_stats;
+
 	if (skb->len > BGMAC_DESC_CTL1_LEN) {
 		netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
 		goto err_drop;
@@ -225,6 +237,7 @@  static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 
 err_drop:
 	dev_kfree_skb(skb);
+err_stats:
 	net_dev->stats.tx_dropped++;
 	net_dev->stats.tx_errors++;
 	return NETDEV_TX_OK;