From 87671375108625bb7f8a09f0809a369d460ebe43 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sat, 11 May 2019 23:14:45 +0300 Subject: [PATCH 1/3] net: dsa: Initialize DSA_SKB_CB(skb)->deferred_xmit variable The sk_buff control block can have any contents on xmit put there by the stack, so initialization is mandatory, since we are checking its value after the actual DSA xmit (the tagger may have changed it). The DSA_SKB_ZERO() macro could have been used for this purpose, but: - Zeroizing a 48-byte memory region in the hotpath is best avoided. - It would have triggered a warning with newer compilers since __dsa_skb_cb contains a structure within a structure, and the {0} initializer was incorrect for that purpose. So simply remove the DSA_SKB_ZERO() macro and initialize the deferred_xmit variable by hand (which should be done for all further dsa_skb_cb variables which need initialization - currently none - to avoid the performance penalty). Fixes: 97a69a0dea9a ("net: dsa: Add support for deferred xmit") Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- include/net/dsa.h | 3 --- net/dsa/slave.c | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 6aaaadd6a413..35ca1f2c6e28 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -102,9 +102,6 @@ struct __dsa_skb_cb { #define DSA_SKB_CB_COPY(nskb, skb) \ { *__DSA_SKB_CB(nskb) = *__DSA_SKB_CB(skb); } -#define DSA_SKB_CB_ZERO(skb) \ - { *__DSA_SKB_CB(skb) = (struct __dsa_skb_cb) {0}; } - #define DSA_SKB_CB_PRIV(skb) \ ((void *)(skb)->cb + offsetof(struct __dsa_skb_cb, priv)) diff --git a/net/dsa/slave.c b/net/dsa/slave.c index fe7b6a62e8f1..9892ca1f6859 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -463,6 +463,8 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev) s->tx_bytes += skb->len; u64_stats_update_end(&s->syncp); + DSA_SKB_CB(skb)->deferred_xmit = false; + /* Identify PTP protocol packets, clone them, and pass them to the * switch driver */ From 506f0e09ce36477ac610b6e9b52cbd4d5ead0c01 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sat, 11 May 2019 23:14:46 +0300 Subject: [PATCH 2/3] net: dsa: Remove dangerous DSA_SKB_CLONE() macro This does not cause any bug now because it has no users, but its body contains two pointer definitions within a code block: struct sk_buff *clone = _clone; \ struct sk_buff *skb = _skb; \ When calling the macro as DSA_SKB_CLONE(clone, skb), these variables would obscure the arguments that the macro was called with, and the initializers would be a no-op instead of doing their job (undefined behavior, by the way, but GCC nicely puts NULL pointers instead). So simply remove this broken macro and leave users to simply call "DSA_SKB_CB(skb)->clone = clone" by hand when needed. There is one functional difference when doing what I just suggested above: the control block won't be transferred from the original skb into the clone. Since there's no foreseen need for the control block in the clone ATM, this is ok. Fixes: b68b0dd0fb2d ("net: dsa: Keep private info in the skb->cb") Signed-off-by: Vladimir Oltean Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- include/net/dsa.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 35ca1f2c6e28..1f6b8608b0b7 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -105,15 +105,6 @@ struct __dsa_skb_cb { #define DSA_SKB_CB_PRIV(skb) \ ((void *)(skb)->cb + offsetof(struct __dsa_skb_cb, priv)) -#define DSA_SKB_CB_CLONE(_clone, _skb) \ - { \ - struct sk_buff *clone = _clone; \ - struct sk_buff *skb = _skb; \ - \ - DSA_SKB_CB_COPY(clone, skb); \ - DSA_SKB_CB(skb)->clone = clone; \ - } - struct dsa_switch_tree { struct list_head list; From 1c9b1420ac137a0060dc67d3840bdaae9bcf4bae Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sat, 11 May 2019 23:14:47 +0300 Subject: [PATCH 3/3] net: dsa: Remove the now unused DSA_SKB_CB_COPY() macro It's best to not expose this, due to the performance hit it may cause when calling it. Fixes: b68b0dd0fb2d ("net: dsa: Keep private info in the skb->cb") Signed-off-by: Vladimir Oltean Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- include/net/dsa.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 1f6b8608b0b7..685294817712 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -99,9 +99,6 @@ struct __dsa_skb_cb { #define DSA_SKB_CB(skb) ((struct dsa_skb_cb *)((skb)->cb)) -#define DSA_SKB_CB_COPY(nskb, skb) \ - { *__DSA_SKB_CB(nskb) = *__DSA_SKB_CB(skb); } - #define DSA_SKB_CB_PRIV(skb) \ ((void *)(skb)->cb + offsetof(struct __dsa_skb_cb, priv))