On Sun, Oct 18, 2020 at 12:36:00AM +0300, Vladimir Oltean wrote:
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d4326940233c..790f5c8deb13 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -548,6 +548,36 @@ netdev_tx_t dsa_enqueue_skb(struct sk_buff *skb, struct
net_device *dev)
}
EXPORT_SYMBOL_GPL(dsa_enqueue_skb);
+static int dsa_realloc_skb(struct sk_buff *skb, struct net_device *dev)
I forgot to actually pad the skb here, if it's a tail tagger, silly me.
The following changes should do the trick.
+{
+ struct net_device *master = dsa_slave_to_master(dev);
The addition of master->needed_headroom and master->needed_tailroom used
to be here, that's why this unused variable is here.
+ struct dsa_slave_priv *p = netdev_priv(dev);
+ struct dsa_slave_stats *e;
+ int headroom, tailroom;
int padlen = 0, err;
+
+ headroom = dev->needed_headroom;
+ tailroom = dev->needed_tailroom;
+ /* For tail taggers, we need to pad short frames ourselves, to ensure
+ * that the tail tag does not fail at its role of being at the end of
+ * the packet, once the master interface pads the frame.
+ */
+ if (unlikely(tailroom && skb->len < ETH_ZLEN))
+ tailroom += ETH_ZLEN - skb->len;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
padlen = ETH_ZLEN - skb->len;
tailroom += padlen;
+
+ if (likely(skb_headroom(skb) >= headroom &&
+ skb_tailroom(skb) >= tailroom) &&
+ !skb_cloned(skb))
+ /* No reallocation needed, yay! */
+ return 0;
+
+ e = this_cpu_ptr(p->extra_stats);
+ u64_stats_update_begin(&e->syncp);
+ e->tx_reallocs++;
+ u64_stats_update_end(&e->syncp);
+
+ return pskb_expand_head(skb, headroom, tailroom, GFP_ATOMIC);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
err = pskb_expand_head(skb, headroom, tailroom, GFP_ATOMIC);
if (err < 0 || !padlen)
return err;
return __skb_put_padto(skb, padlen, false);
+}
+
static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
@@ -567,6 +597,11 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb,
struct net_device *dev)
*/
dsa_skb_tx_timestamp(p, skb);
+ if (dsa_realloc_skb(skb, dev)) {
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
+
/* Transmit function may have to reallocate the original SKB,
* in which case it must have freed it. Only free it here on error.
*/
@@ -1802,6 +1837,14 @@ int dsa_slave_create(struct dsa_port *port)
slave_dev->netdev_ops = &dsa_slave_netdev_ops;
if (ds->ops->port_max_mtu)
slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
+ /* Try to save one extra realloc later in the TX path (in the master)
+ * by also inheriting the master's needed headroom and tailroom.
+ * The 8021q driver also does this.
+ */
Also, this comment is bogus given the current code. It should be removed
from here, and...
+ if (cpu_dp->tag_ops->tail_tag)
+ slave_dev->needed_tailroom = cpu_dp->tag_ops->overhead;
+ else
+ slave_dev->needed_headroom = cpu_dp->tag_ops->overhead;
...put here, along with:
slave_dev->needed_headroom += master->needed_headroom;
slave_dev->needed_tailroom += master->needed_tailroom;
