* [i40e] regression on TCP stream and TCP maerts, kernel-4.12.0-0.rc2
From: Adam Okuliar @ 2017-05-30 13:43 UTC (permalink / raw)
To: netdev, Adrian Tomasov, jeffrey.t.kirsher; +Cc: Otto Sabart, jhladky
Hello,
we found regression on intel card(XL710) with i40e driver. Regression is
about ~45%
on TCP_STREAM and TCP_MAERTS test for IPv4 and IPv6. Regression was first
visible in kernel-4.12.0-0.rc1.
More details about results you can see in uploaded images in bugzilla. [0]
[0] https://bugzilla.kernel.org/show_bug.cgi?id=195923
Best regards, / S pozdravom,
Adrián Tomašov
Kernel Performance QE
atomasov@redhat.com
^ permalink raw reply
* [PATCH net-next] rtnetlink: use the new rtnl_get_event() interface
From: Vladislav Yasevich @ 2017-05-30 14:07 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Vladislav Yasevich
Small clean-up to rtmsg_ifinfo() to use the rtnl_get_event()
interface instead of using 'internal' values directly.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/core/rtnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9da53e4..7084f1d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2896,7 +2896,7 @@ static void rtmsg_ifinfo_event(int type, struct net_device *dev,
void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
gfp_t flags)
{
- rtmsg_ifinfo_event(type, dev, change, IFLA_EVENT_NONE, flags);
+ rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags);
}
EXPORT_SYMBOL(rtmsg_ifinfo);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] rtnetlink: use the new rtnl_get_event() interface
From: David Ahern @ 2017-05-30 14:14 UTC (permalink / raw)
To: Vladislav Yasevich, netdev; +Cc: Vladislav Yasevich
In-Reply-To: <1496153222-13382-1-git-send-email-vyasevic@redhat.com>
On 5/30/17 8:07 AM, Vladislav Yasevich wrote:
> Small clean-up to rtmsg_ifinfo() to use the rtnl_get_event()
> interface instead of using 'internal' values directly.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> net/core/rtnetlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [PATCH net-next 1/7] net: dsa: hide dsa_uses_tagged_protocol code
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
Hide the implementation of dsa_uses_tagged_protocol in dsa.c since this
helper will be extended to access the opaque dsa_device_ops structure.
At the same time, fix the checkpatch comparison check:
CHECK: Comparison to NULL could be written "dst->rcv"
#41: FILE: net/dsa/dsa.c:32:
+ return dst->rcv != NULL;
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 5 +----
net/dsa/dsa.c | 5 +++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index c0e567c0c824..cb5d668b265d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -464,10 +464,7 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
struct net_device *dsa_dev_to_net_device(struct device *dev);
-static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
-{
- return dst->rcv != NULL;
-}
+bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst);
static inline bool netdev_uses_dsa(struct net_device *dev)
{
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3288a80d4d6c..7a8a0358299b 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -27,6 +27,11 @@
#include "dsa_priv.h"
+bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
+{
+ return !!dst->rcv;
+}
+
static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff *skb,
struct net_device *dev)
{
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 3/7] net: dsa: remove unused arguments of tagger rcv
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
The struct dsa_device_ops defines the rcv function with 2 unused
arguments struct packet_type *pt, and struct net_device *orig_dev.
This patch removes them from the definition and implementations.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa.c | 2 +-
net/dsa/dsa_priv.h | 4 +---
net/dsa/tag_brcm.c | 4 +---
net/dsa/tag_dsa.c | 4 +---
net/dsa/tag_edsa.c | 4 +---
net/dsa/tag_lan9303.c | 3 +--
net/dsa/tag_mtk.c | 4 +---
net/dsa/tag_qca.c | 4 +---
net/dsa/tag_trailer.c | 4 +---
9 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 861dc0e5020d..98173a3f6fd1 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -209,7 +209,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
if (!skb)
return 0;
- nskb = dst->tag_ops->rcv(skb, dev, pt, orig_dev);
+ nskb = dst->tag_ops->rcv(skb, dev);
if (!nskb) {
kfree_skb(skb);
return 0;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index c1d4180651af..0fdd2a8a4ad8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -67,9 +67,7 @@ struct dsa_notifier_vlan_info {
struct dsa_device_ops {
struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
- struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev);
+ struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
};
struct dsa_slave_priv {
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 9f204f18ada3..c8bff4d7543d 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -92,9 +92,7 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
return NULL;
}
-static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 3b62a57956a3..41f78e42f9ba 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -68,9 +68,7 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
return NULL;
}
-static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index f95cafd05702..36389fc8fe06 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -81,9 +81,7 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
return NULL;
}
-static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index afd59330b5f1..82e150486497 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -71,8 +71,7 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
return NULL;
}
-static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev)
{
u16 *lan9303_tag;
struct dsa_switch_tree *dst = dev->dsa_ptr;
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index d1258e84cd71..7c9764c8d61b 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -47,9 +47,7 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
return NULL;
}
-static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 2451007699b7..d38bd5cbfad8 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -66,9 +66,7 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
return NULL;
}
-static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 7488ab2932ab..e1fd78197fc8 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -58,9 +58,7 @@ static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
return nskb;
}
-static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev)
+static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_switch *ds;
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 5/7] net: dsa: remove useless goto label in tagger rcv
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
Many rcv functions from net/dsa/tag_*.c have a useless out_drop goto
label which simply returns NULL. Kill it in favor of the obvious.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/tag_brcm.c | 11 ++++-------
net/dsa/tag_dsa.c | 13 +++++--------
net/dsa/tag_edsa.c | 13 +++++--------
net/dsa/tag_mtk.c | 9 +++------
net/dsa/tag_qca.c | 11 ++++-------
net/dsa/tag_trailer.c | 9 +++------
6 files changed, 24 insertions(+), 42 deletions(-)
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index c8bff4d7543d..10fa4c0ca46b 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -102,27 +102,27 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev)
ds = dst->cpu_dp->ds;
if (unlikely(!pskb_may_pull(skb, BRCM_TAG_LEN)))
- goto out_drop;
+ return NULL;
/* skb->data points to the EtherType, the tag is right before it */
brcm_tag = skb->data - 2;
/* The opcode should never be different than 0b000 */
if (unlikely((brcm_tag[0] >> BRCM_OPCODE_SHIFT) & BRCM_OPCODE_MASK))
- goto out_drop;
+ return NULL;
/* We should never see a reserved reason code without knowing how to
* handle it
*/
if (unlikely(brcm_tag[2] & BRCM_EG_RC_RSVD))
- goto out_drop;
+ return NULL;
/* Locate which port this is coming from */
source_port = brcm_tag[3] & BRCM_EG_PID_MASK;
/* Validate port against switch setup, either the port is totally */
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/* Remove Broadcom tag and update checksum */
skb_pull_rcsum(skb, BRCM_TAG_LEN);
@@ -135,9 +135,6 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev)
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops brcm_netdev_ops = {
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 41f78e42f9ba..9f5fecaa4a93 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -77,7 +77,7 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
int source_port;
if (unlikely(!pskb_may_pull(skb, DSA_HLEN)))
- goto out_drop;
+ return NULL;
/*
* The ethertype field is part of the DSA header.
@@ -88,7 +88,7 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
* Check that frame type is either TO_CPU or FORWARD.
*/
if ((dsa_header[0] & 0xc0) != 0x00 && (dsa_header[0] & 0xc0) != 0xc0)
- goto out_drop;
+ return NULL;
/*
* Determine source device and port.
@@ -101,14 +101,14 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
* port is a registered DSA port.
*/
if (source_device >= DSA_MAX_SWITCHES)
- goto out_drop;
+ return NULL;
ds = dst->ds[source_device];
if (!ds)
- goto out_drop;
+ return NULL;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/*
* Convert the DSA header to an 802.1q header if the 'tagged'
@@ -159,9 +159,6 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops dsa_netdev_ops = {
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 36389fc8fe06..a9a06e19abeb 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -90,7 +90,7 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
int source_port;
if (unlikely(!pskb_may_pull(skb, EDSA_HLEN)))
- goto out_drop;
+ return NULL;
/*
* Skip the two null bytes after the ethertype.
@@ -101,7 +101,7 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
* Check that frame type is either TO_CPU or FORWARD.
*/
if ((edsa_header[0] & 0xc0) != 0x00 && (edsa_header[0] & 0xc0) != 0xc0)
- goto out_drop;
+ return NULL;
/*
* Determine source device and port.
@@ -114,14 +114,14 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
* port is a registered DSA port.
*/
if (source_device >= DSA_MAX_SWITCHES)
- goto out_drop;
+ return NULL;
ds = dst->ds[source_device];
if (!ds)
- goto out_drop;
+ return NULL;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/*
* If the 'tagged' bit is set, convert the DSA tag to a 802.1q
@@ -178,9 +178,6 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops edsa_netdev_ops = {
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 7c9764c8d61b..327dd7b596df 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -55,7 +55,7 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev)
__be16 *phdr, hdr;
if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN)))
- goto out_drop;
+ return NULL;
/* The MTK header is added by the switch between src addr
* and ethertype at this point, skb->data points to 2 bytes
@@ -77,19 +77,16 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev)
*/
ds = dst->ds[0];
if (!ds)
- goto out_drop;
+ return NULL;
/* Get source port information */
port = (hdr & MTK_HDR_RECV_SOURCE_PORT_MASK);
if (!ds->ports[port].netdev)
- goto out_drop;
+ return NULL;
skb->dev = ds->ports[port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops mtk_netdev_ops = {
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index d38bd5cbfad8..8080ad8f2abb 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -75,7 +75,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
__be16 *phdr, hdr;
if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
- goto out_drop;
+ return NULL;
/* The QCA header is added by the switch between src addr and Ethertype
* At this point, skb->data points to ethertype so header should be
@@ -87,7 +87,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
/* Make sure the version is correct */
ver = (hdr & QCA_HDR_RECV_VERSION_MASK) >> QCA_HDR_RECV_VERSION_S;
if (unlikely(ver != QCA_HDR_VERSION))
- goto out_drop;
+ return NULL;
/* Remove QCA tag and recalculate checksum */
skb_pull_rcsum(skb, QCA_HDR_LEN);
@@ -99,20 +99,17 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
*/
ds = dst->cpu_dp->ds;
if (!ds)
- goto out_drop;
+ return NULL;
/* Get source port information */
port = (hdr & QCA_HDR_RECV_SOURCE_PORT_MASK);
if (!ds->ports[port].netdev)
- goto out_drop;
+ return NULL;
/* Update skb & forward the frame accordingly */
skb->dev = ds->ports[port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops qca_netdev_ops = {
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index e1fd78197fc8..7422b1329712 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -68,25 +68,22 @@ static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev)
ds = dst->cpu_dp->ds;
if (skb_linearize(skb))
- goto out_drop;
+ return NULL;
trailer = skb_tail_pointer(skb) - 4;
if (trailer[0] != 0x80 || (trailer[1] & 0xf8) != 0x00 ||
(trailer[2] & 0xef) != 0x00 || trailer[3] != 0x00)
- goto out_drop;
+ return NULL;
source_port = trailer[1] & 7;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
pskb_trim_rcsum(skb, skb->len - 4);
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops trailer_netdev_ops = {
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 6/7] net: dsa: remove useless copy of tagger xmit
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
The dsa_slave_priv structure holds a copy of the dsa_device_ops xmit
function. It is always assigned to the switch tree xmit function.
Remove this useless copy.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa_priv.h | 3 ---
net/dsa/slave.c | 4 +---
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 0fdd2a8a4ad8..282a55639285 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -71,9 +71,6 @@ struct dsa_device_ops {
};
struct dsa_slave_priv {
- struct sk_buff * (*xmit)(struct sk_buff *skb,
- struct net_device *dev);
-
/* DSA port data, such as switch, port index, etc. */
struct dsa_port *dp;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 887e26695519..ebad2af4d3a8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -363,7 +363,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_bytes += skb->len;
/* Transmit function may have to reallocate the original SKB */
- nskb = p->xmit(skb, dev);
+ nskb = p->dp->ds->dst->tag_ops->xmit(skb, dev);
if (!nskb)
return NETDEV_TX_OK;
@@ -1136,7 +1136,6 @@ int dsa_slave_resume(struct net_device *slave_dev)
int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
int port, const char *name)
{
- struct dsa_switch_tree *dst = ds->dst;
struct net_device *master;
struct net_device *slave_dev;
struct dsa_slave_priv *p;
@@ -1172,7 +1171,6 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
p = netdev_priv(slave_dev);
p->dp = &ds->ports[port];
INIT_LIST_HEAD(&p->mall_tc_list);
- p->xmit = dst->tag_ops->xmit;
p->old_pause = -1;
p->old_link = -1;
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 7/7] net: dsa: factor skb freeing on xmit
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
The taggers are currently responsible to free the original SKB if they
made a copy of it, or in case of error.
This patch simplifies this by freeing the original SKB in the
dsa_slave_xmit caller if it differs from the return SKB (copy or NULL.)
At the same time, fix the checkpatch NULL comparison check:
CHECK: Comparison to NULL could be written "!nskb"
#208: FILE: net/dsa/tag_trailer.c:35:
+ if (nskb == NULL)
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 2 ++
net/dsa/tag_brcm.c | 6 +-----
net/dsa/tag_dsa.c | 8 ++------
net/dsa/tag_edsa.c | 8 ++------
net/dsa/tag_lan9303.c | 5 +----
net/dsa/tag_mtk.c | 6 +-----
net/dsa/tag_qca.c | 6 +-----
net/dsa/tag_trailer.c | 5 +----
8 files changed, 11 insertions(+), 35 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ebad2af4d3a8..a520c822436e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -364,6 +364,8 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
/* Transmit function may have to reallocate the original SKB */
nskb = p->dp->ds->dst->tag_ops->xmit(skb, dev);
+ if (nskb != skb)
+ kfree_skb(skb);
if (!nskb)
return NETDEV_TX_OK;
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 10fa4c0ca46b..5db6bcfde025 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -65,7 +65,7 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
u8 *brcm_tag;
if (skb_cow_head(skb, BRCM_TAG_LEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, BRCM_TAG_LEN);
@@ -86,10 +86,6 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
brcm_tag[3] = (1 << p->dp->index) & BRCM_IG_DSTMAP1_MASK;
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 9f5fecaa4a93..6837ca9160a8 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -28,7 +28,7 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
*/
if (skb->protocol == htons(ETH_P_8021Q)) {
if (skb_cow_head(skb, 0) < 0)
- goto out_free;
+ return NULL;
/*
* Construct tagged FROM_CPU DSA tag from 802.1q tag.
@@ -46,7 +46,7 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
} else {
if (skb_cow_head(skb, DSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, DSA_HLEN);
memmove(skb->data, skb->data + DSA_HLEN, 2 * ETH_ALEN);
@@ -62,10 +62,6 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index a9a06e19abeb..96ddc90472a2 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -30,7 +30,7 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
*/
if (skb->protocol == htons(ETH_P_8021Q)) {
if (skb_cow_head(skb, DSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, DSA_HLEN);
memmove(skb->data, skb->data + DSA_HLEN, 2 * ETH_ALEN);
@@ -55,7 +55,7 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
} else {
if (skb_cow_head(skb, EDSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, EDSA_HLEN);
memmove(skb->data, skb->data + EDSA_HLEN, 2 * ETH_ALEN);
@@ -75,10 +75,6 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 82e150486497..e33348101e8f 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -52,7 +52,7 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
if (skb_cow_head(skb, LAN9303_TAG_LEN) < 0) {
dev_dbg(&dev->dev,
"Cannot make room for the special tag. Dropping packet\n");
- goto out_free;
+ return NULL;
}
/* provide 'LAN9303_TAG_LEN' bytes additional space */
@@ -66,9 +66,6 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
lan9303_tag[1] = htons(p->dp->index | BIT(4));
return skb;
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 327dd7b596df..ee62ae5f03e1 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -27,7 +27,7 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
u8 *mtk_tag;
if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, MTK_HDR_LEN);
@@ -41,10 +41,6 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
mtk_tag[3] = 0;
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 8080ad8f2abb..4d3bc2920477 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -45,7 +45,7 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_bytes += skb->len;
if (skb_cow_head(skb, 0) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, QCA_HDR_LEN);
@@ -60,10 +60,6 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
*phdr = htons(hdr);
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 7422b1329712..05dafd7cdb21 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -32,17 +32,14 @@ static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
padlen = 60 - skb->len;
nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 4, GFP_ATOMIC);
- if (nskb == NULL) {
- kfree_skb(skb);
+ if (!nskb)
return NULL;
- }
skb_reserve(nskb, NET_IP_ALIGN);
skb_reset_mac_header(nskb);
skb_set_network_header(nskb, skb_network_header(skb) - skb->head);
skb_set_transport_header(nskb, skb_transport_header(skb) - skb->head);
skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
- kfree_skb(skb);
if (padlen) {
u8 *pad = skb_put(nskb, padlen);
--
2.13.0
^ permalink raw reply related
* Re: [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor
From: David Ahern @ 2017-05-30 14:22 UTC (permalink / raw)
To: Vladislav Yasevich, netdev; +Cc: roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495894476-9726-4-git-send-email-vyasevic@redhat.com>
On 5/27/17 8:14 AM, Vladislav Yasevich wrote:
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index b8d9c7d..c6e7413 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
> return 0;
> }
>
> +static const char *netdev_events[] = {"NONE",
I would prefer ifla_events rather than netdev just to reinforce the
separation that an IFLA_EVENT does not necessarily correspond to a
NETDEV event.
^ permalink raw reply
* [PATCH net-next 4/7] net: dsa: free orig skb on rcv if reallocated
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
If the receive function of a tagger reallocated the SKB, the original
SKB is currently not freed. Fix this and free it on both copy or error.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 98173a3f6fd1..0b6f2c7d7d1c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -209,11 +209,12 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
if (!skb)
return 0;
+ /* Receive function may have to reallocate the original SKB */
nskb = dst->tag_ops->rcv(skb, dev);
- if (!nskb) {
+ if (nskb != skb)
kfree_skb(skb);
+ if (!nskb)
return 0;
- }
skb = nskb;
skb_push(skb, ETH_HLEN);
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 0/7] net: dsa: tagger simplification
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
The DSA layer has a dsa_device_ops structure containing two members to
tag and untag the proprietary switch header, called xmit and rcv.
The switch tree and slave private structures respectively hold a useless
copy of the rcv and xmit functions. The tagging implementations use
useless goto labels and only the rcv caller (partially) handles the SKB
freeing. The rcv function also contains unused arguments.
This patchset removes the copy of rcv and xmit functions, the unused
arguments of the rcv signature and the useless labels, and handles
freeing of the SKB in the xmit caller.
Vivien Didelot (7):
net: dsa: hide dsa_uses_tagged_protocol code
net: dsa: remove useless rcv copy in DSA tree
net: dsa: remove unused arguments of tagger rcv
net: dsa: free orig skb on rcv if reallocated
net: dsa: remove useless goto label in tagger rcv
net: dsa: remove useless copy of tagger xmit
net: dsa: factor skb freeing on xmit
include/net/dsa.h | 9 +--------
net/dsa/dsa.c | 12 +++++++++---
net/dsa/dsa2.c | 2 --
net/dsa/dsa_priv.h | 7 +------
net/dsa/legacy.c | 2 --
net/dsa/slave.c | 6 +++---
net/dsa/tag_brcm.c | 21 ++++++---------------
net/dsa/tag_dsa.c | 25 ++++++++-----------------
net/dsa/tag_edsa.c | 25 ++++++++-----------------
net/dsa/tag_lan9303.c | 8 ++------
net/dsa/tag_mtk.c | 19 +++++--------------
net/dsa/tag_qca.c | 21 ++++++---------------
net/dsa/tag_trailer.c | 18 +++++-------------
13 files changed, 54 insertions(+), 121 deletions(-)
--
2.13.0
^ permalink raw reply
* [PATCH net-next 2/7] net: dsa: remove useless rcv copy in DSA tree
From: Vivien Didelot @ 2017-05-30 14:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
The dsa_switch_tree holds a copy of the rcv member of the dsa_device_ops
structure. dst->rcv is always assigned to dst->tag_ops->rcv. Remove this
useless copy.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 4 ----
net/dsa/dsa.c | 4 ++--
net/dsa/dsa2.c | 2 --
net/dsa/legacy.c | 2 --
4 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index cb5d668b265d..4b82766715e9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -126,10 +126,6 @@ struct dsa_switch_tree {
* protocol to use.
*/
struct net_device *master_netdev;
- struct sk_buff * (*rcv)(struct sk_buff *skb,
- struct net_device *dev,
- struct packet_type *pt,
- struct net_device *orig_dev);
/*
* Original copy of the master netdev ethtool_ops
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 7a8a0358299b..861dc0e5020d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -29,7 +29,7 @@
bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
{
- return !!dst->rcv;
+ return dst->tag_ops && dst->tag_ops->rcv;
}
static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff *skb,
@@ -209,7 +209,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
if (!skb)
return 0;
- nskb = dst->rcv(skb, dev, pt, orig_dev);
+ nskb = dst->tag_ops->rcv(skb, dev, pt, orig_dev);
if (!nskb) {
kfree_skb(skb);
return 0;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4301f52e4f5a..12306c15ed51 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -528,8 +528,6 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
return PTR_ERR(dst->tag_ops);
}
- dst->rcv = dst->tag_ops->rcv;
-
return 0;
}
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index ac4379b8d7ac..738c5c2d7d3c 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -150,8 +150,6 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
dst->tag_ops = dsa_resolve_tag_protocol(tag_protocol);
if (IS_ERR(dst->tag_ops))
return PTR_ERR(dst->tag_ops);
-
- dst->rcv = dst->tag_ops->rcv;
}
memcpy(ds->rtable, cd->rtable, sizeof(ds->rtable));
--
2.13.0
^ permalink raw reply related
* Re: [PATCH net-next 1/7] net: dsa: hide dsa_uses_tagged_protocol code
From: Andrew Lunn @ 2017-05-30 15:01 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-2-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:25AM -0400, Vivien Didelot wrote:
> Hide the implementation of dsa_uses_tagged_protocol in dsa.c since this
> helper will be extended to access the opaque dsa_device_ops structure.
>
> At the same time, fix the checkpatch comparison check:
>
> CHECK: Comparison to NULL could be written "dst->rcv"
> #41: FILE: net/dsa/dsa.c:32:
> + return dst->rcv != NULL;
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> include/net/dsa.h | 5 +----
> net/dsa/dsa.c | 5 +++++
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index c0e567c0c824..cb5d668b265d 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -464,10 +464,7 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
>
> struct net_device *dsa_dev_to_net_device(struct device *dev);
>
> -static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
> -{
> - return dst->rcv != NULL;
> -}
> +bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst);
>
> static inline bool netdev_uses_dsa(struct net_device *dev)
> {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3288a80d4d6c..7a8a0358299b 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -27,6 +27,11 @@
>
> #include "dsa_priv.h"
>
> +bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
> +{
> + return !!dst->rcv;
> +}
> +
Hi Vivien
You need to be careful here. This is in the hot path. Every frame
received uses this code. And think about a distro kernel, which might
have DSA enabled by default, yet is unlikely to have any switches. You
are adding a function call which can be called millions of times per
second....
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/7] net: dsa: remove useless rcv copy in DSA tree
From: Andrew Lunn @ 2017-05-30 15:05 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-3-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:26AM -0400, Vivien Didelot wrote:
> The dsa_switch_tree holds a copy of the rcv member of the dsa_device_ops
> structure. dst->rcv is always assigned to dst->tag_ops->rcv. Remove this
> useless copy.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> include/net/dsa.h | 4 ----
> net/dsa/dsa.c | 4 ++--
> net/dsa/dsa2.c | 2 --
> net/dsa/legacy.c | 2 --
> 4 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index cb5d668b265d..4b82766715e9 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -126,10 +126,6 @@ struct dsa_switch_tree {
> * protocol to use.
> */
> struct net_device *master_netdev;
> - struct sk_buff * (*rcv)(struct sk_buff *skb,
> - struct net_device *dev,
> - struct packet_type *pt,
> - struct net_device *orig_dev);
>
> /*
> * Original copy of the master netdev ethtool_ops
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 7a8a0358299b..861dc0e5020d 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -29,7 +29,7 @@
>
> bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
> {
> - return !!dst->rcv;
> + return dst->tag_ops && dst->tag_ops->rcv;
This makes the hot path more expensive. The copy is probably worth it
in terms of performance.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 0/7] net: dsa: tagger simplification
From: Andrew Lunn @ 2017-05-30 15:08 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-1-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:24AM -0400, Vivien Didelot wrote:
> The DSA layer has a dsa_device_ops structure containing two members to
> tag and untag the proprietary switch header, called xmit and rcv.
>
> The switch tree and slave private structures respectively hold a useless
> copy of the rcv and xmit functions. The tagging implementations use
> useless goto labels and only the rcv caller (partially) handles the SKB
> freeing. The rcv function also contains unused arguments.
>
> This patchset removes the copy of rcv and xmit functions, the unused
> arguments of the rcv signature and the useless labels, and handles
> freeing of the SKB in the xmit caller.
Hi Vivien
Since these changes are dealing with the hot path of the network
stack, it would be good to see some performance results. In
particular, from a high performance machine, 10Gbps interfaces, where
DSA is enabled, as a distro kernel might have.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/7] net: dsa: hide dsa_uses_tagged_protocol code
From: David Miller @ 2017-05-30 15:09 UTC (permalink / raw)
To: andrew; +Cc: vivien.didelot, netdev, linux-kernel, kernel, f.fainelli
In-Reply-To: <20170530150144.GC22758@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 30 May 2017 17:01:44 +0200
> On Tue, May 30, 2017 at 10:21:25AM -0400, Vivien Didelot wrote:
>> Hide the implementation of dsa_uses_tagged_protocol in dsa.c since this
>> helper will be extended to access the opaque dsa_device_ops structure.
>>
>> At the same time, fix the checkpatch comparison check:
>>
>> CHECK: Comparison to NULL could be written "dst->rcv"
>> #41: FILE: net/dsa/dsa.c:32:
>> + return dst->rcv != NULL;
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>> include/net/dsa.h | 5 +----
>> net/dsa/dsa.c | 5 +++++
>> 2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index c0e567c0c824..cb5d668b265d 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -464,10 +464,7 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
>>
>> struct net_device *dsa_dev_to_net_device(struct device *dev);
>>
>> -static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
>> -{
>> - return dst->rcv != NULL;
>> -}
>> +bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst);
>>
>> static inline bool netdev_uses_dsa(struct net_device *dev)
>> {
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 3288a80d4d6c..7a8a0358299b 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -27,6 +27,11 @@
>>
>> #include "dsa_priv.h"
>>
>> +bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
>> +{
>> + return !!dst->rcv;
>> +}
>> +
>
> Hi Vivien
>
> You need to be careful here. This is in the hot path. Every frame
> received uses this code. And think about a distro kernel, which might
> have DSA enabled by default, yet is unlikely to have any switches. You
> are adding a function call which can be called millions of times per
> second....
Yeah, we really can't make this change.
This isn't glibc where we're trying to hide the implementation of "FILE *"
behind accessor functions that caller can't see. We inline things when
performance dictates, and it does here.
^ permalink raw reply
* Re: [PATCH net-next 2/7] net: dsa: remove useless rcv copy in DSA tree
From: David Miller @ 2017-05-30 15:09 UTC (permalink / raw)
To: andrew; +Cc: vivien.didelot, netdev, linux-kernel, kernel, f.fainelli
In-Reply-To: <20170530150531.GD22758@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 30 May 2017 17:05:31 +0200
> On Tue, May 30, 2017 at 10:21:26AM -0400, Vivien Didelot wrote:
>> The dsa_switch_tree holds a copy of the rcv member of the dsa_device_ops
>> structure. dst->rcv is always assigned to dst->tag_ops->rcv. Remove this
>> useless copy.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>> include/net/dsa.h | 4 ----
>> net/dsa/dsa.c | 4 ++--
>> net/dsa/dsa2.c | 2 --
>> net/dsa/legacy.c | 2 --
>> 4 files changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index cb5d668b265d..4b82766715e9 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -126,10 +126,6 @@ struct dsa_switch_tree {
>> * protocol to use.
>> */
>> struct net_device *master_netdev;
>> - struct sk_buff * (*rcv)(struct sk_buff *skb,
>> - struct net_device *dev,
>> - struct packet_type *pt,
>> - struct net_device *orig_dev);
>>
>> /*
>> * Original copy of the master netdev ethtool_ops
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 7a8a0358299b..861dc0e5020d 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -29,7 +29,7 @@
>>
>> bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
>> {
>> - return !!dst->rcv;
>> + return dst->tag_ops && dst->tag_ops->rcv;
>
> This makes the hot path more expensive. The copy is probably worth it
> in terms of performance.
Agreed.
^ permalink raw reply
* Re: [PATCH net-next 3/7] net: dsa: remove unused arguments of tagger rcv
From: Andrew Lunn @ 2017-05-30 15:13 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-4-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:27AM -0400, Vivien Didelot wrote:
> The struct dsa_device_ops defines the rcv function with 2 unused
> arguments struct packet_type *pt, and struct net_device *orig_dev.
>
> This patch removes them from the definition and implementations.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] net: phy: fix marvell phy status reading
From: Russell King @ 2017-05-30 15:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev
The Marvell driver incorrectly provides phydev->lp_advertising as the
logical and of the link partner's advert and our advert. This is
incorrect - this field is supposed to store the link parter's unmodified
advertisment.
This allows ethtool to report the correct link partner auto-negotiation
status.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index e38f84291258..f93a7a46feb7 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1119,8 +1119,6 @@ static int marvell_read_status_page(struct phy_device *phydev, int page)
if (adv < 0)
return adv;
- lpa &= adv;
-
if (status & MII_M1011_PHY_STATUS_FULLDUPLEX)
phydev->duplex = DUPLEX_FULL;
else
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 4/7] net: dsa: free orig skb on rcv if reallocated
From: Andrew Lunn @ 2017-05-30 15:25 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-5-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:28AM -0400, Vivien Didelot wrote:
> If the receive function of a tagger reallocated the SKB, the original
> SKB is currently not freed. Fix this and free it on both copy or error.
I don't see any of the receive functions reallocate the skb. It might
be better to just simplify the code to take away the option to return
a different skb.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 6/7] net: dsa: remove useless copy of tagger xmit
From: Andrew Lunn @ 2017-05-30 15:29 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-7-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:30AM -0400, Vivien Didelot wrote:
> The dsa_slave_priv structure holds a copy of the dsa_device_ops xmit
> function. It is always assigned to the switch tree xmit function.
>
> Remove this useless copy.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> net/dsa/dsa_priv.h | 3 ---
> net/dsa/slave.c | 4 +---
> 2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 0fdd2a8a4ad8..282a55639285 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -71,9 +71,6 @@ struct dsa_device_ops {
> };
>
> struct dsa_slave_priv {
> - struct sk_buff * (*xmit)(struct sk_buff *skb,
> - struct net_device *dev);
> -
> /* DSA port data, such as switch, port index, etc. */
> struct dsa_port *dp;
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 887e26695519..ebad2af4d3a8 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -363,7 +363,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
> dev->stats.tx_bytes += skb->len;
>
> /* Transmit function may have to reallocate the original SKB */
> - nskb = p->xmit(skb, dev);
> + nskb = p->dp->ds->dst->tag_ops->xmit(skb, dev);
This is also the hot path for DSA transmit. Do we really want to do 4
extra pointer dereferences a million times per second, compared to one
copy during setup?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 7/7] net: dsa: factor skb freeing on xmit
From: Andrew Lunn @ 2017-05-30 15:37 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170530142131.23568-8-vivien.didelot@savoirfairelinux.com>
On Tue, May 30, 2017 at 10:21:31AM -0400, Vivien Didelot wrote:
> The taggers are currently responsible to free the original SKB if they
> made a copy of it, or in case of error.
>
> This patch simplifies this by freeing the original SKB in the
> dsa_slave_xmit caller if it differs from the return SKB (copy or NULL.)
So we have two different things here:
1) The tagger needs to drop the frame, because there is an error of
some sort.
2) The tagger allocates a new skb for the frame.
I think on error, the core code can drop the frame.
If the tagger allocates a new skb, i think the tagger should be
responsible for freeing to original.
Andrew
^ permalink raw reply
* Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support
From: kbuild test robot @ 2017-05-30 15:38 UTC (permalink / raw)
To: John Crispin
Cc: kbuild-all, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Sean Wang, netdev, linux-kernel, John Crispin
In-Reply-To: <20170530104419.6052-2-john@phrozen.org>
[-- Attachment #1: Type: text/plain, Size: 6633 bytes --]
Hi John,
[auto build test ERROR on net-next/master]
[also build test ERROR on next-20170530]
[cannot apply to v4.12-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/John-Crispin/Documentation-devicetree-add-multiple-cpu-port-DSA-binding/20170530-224954
config: x86_64-randconfig-x014-201722 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
net//dsa/dsa2.c: In function 'dsa_ds_parse':
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from incompatible pointer type [-Werror=incompatible-pointer-types]
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument is of type 'struct dsa_port *'
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:12: error: too few arguments to function 'dsa_is_normal_port'
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: declared here
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from incompatible pointer type [-Werror=incompatible-pointer-types]
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument is of type 'struct dsa_port *'
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:12: error: too few arguments to function 'dsa_is_normal_port'
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: declared here
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from incompatible pointer type [-Werror=incompatible-pointer-types]
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument is of type 'struct dsa_port *'
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:12: error: too few arguments to function 'dsa_is_normal_port'
else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: declared here
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/dsa_is_normal_port +574 net//dsa/dsa2.c
568 port = &ds->ports[index];
569 if (!dsa_port_is_valid(port))
570 continue;
571
572 if (dsa_port_is_cpu(port))
573 err = dsa_cpu_parse(port, index, dst, ds);
> 574 else if (dsa_is_normal_port(port))
575 err = dsa_user_parse(port->dn, index, ds);
576
577 if (err)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28681 bytes --]
^ permalink raw reply
* Re: [i40e] regression on TCP stream and TCP maerts, kernel-4.12.0-0.rc2
From: Alexander Duyck @ 2017-05-30 15:41 UTC (permalink / raw)
To: Adam Okuliar, Mitch Williams, intel-wired-lan
Cc: Netdev, Adrian Tomasov, Jeff Kirsher, Otto Sabart, Jirka Hladky
In-Reply-To: <655ed9ab-853e-7497-9ea3-cc8d13f56043@redhat.com>
On Tue, May 30, 2017 at 6:43 AM, Adam Okuliar <aokuliar@redhat.com> wrote:
> Hello,
>
> we found regression on intel card(XL710) with i40e driver. Regression is
> about ~45%
> on TCP_STREAM and TCP_MAERTS test for IPv4 and IPv6. Regression was first
> visible in kernel-4.12.0-0.rc1.
>
> More details about results you can see in uploaded images in bugzilla. [0]
>
>
> [0] https://bugzilla.kernel.org/show_bug.cgi?id=195923
>
>
> Best regards, / S pozdravom,
>
> Adrián Tomašov
> Kernel Performance QE
> atomasov@redhat.com
I have added the i40e driver maintainer and the intel-wired-lan
mailing list so that we can make are developers aware of the issue.
Thanks.
- Alex
^ permalink raw reply
* [PATCH v4 net-next 0/5] dsa: add Microchip KSZ9477 DSA driver
From: Woojung.Huh @ 2017-05-30 15:42 UTC (permalink / raw)
To: andrew, f.fainelli, netdev; +Cc: davem, UNGLinuxDriver
From: Woojung Huh <Woojung.Huh@microchip.com>
This series of patches is for Microchip KSZ9477 DSA driver.
KSZ9477 is 7 ports GigE switch with numerous advanced features.
5 ports are 10/100/1000 Mbps internal PHYs and 2 ports have
Interfaces to SGMII, RGMII, MII or RMII.
This patch supports VLAN, MDB, FDB and port mirroring offloads.
Welcome reviews and comments from community.
Note: Tests are performed on internal development board.
V4
- update per review comments
- cosmetic changes
- net/dsa/tag_ksz.c
* skb_put() & memset() are changed to skb_put_padto()
- drivers/net/dsa/microchip/ksz_common.
* vlan access mutex is updated
* mib_names[] is changed to static const
V3
- update per review comments
- cosmetic changes
- drivers/net/dsa/microchip/ksz_common.c
* clean up ksz_switch_chips[]
* consolidate checking loops into functions
* update mutex for better locking
* replace devm_kmalloc_array() to devm_kcalloc()
- MAINTAINERS
* add missing net/dsa/tag_ksz.c
V2
- update per review comments
- several cosmetic changes
- net/dsa/tag_ksz.c
* constants are changed to defines
* remove skb_linearize() in ksz_rcv()
* ksz_xmit()checks skb tailroom before allocate new skb
- drivers/net/phy/micrel.c
* remove PHY_HAS_MAGICANEG from ksphy_driver[]
- drivers/net/dsa/microchip/ksz_common.c
* add timeout to avoid endless loop
* port initialization is move to ksz_port_enable() instead of ksz_setup_ports()
- Documentation/devicetree/bindings/net/dsa/ksz.txt
* fix typo and indentations
Woojung Huh (5):
dsa: add support for Microchip KSZ tail tagging
phy: micrel: add Microchip KSZ 9477 Switch PHY support
dsa: add DSA switch driver for Microchip KSZ9477
nex: dsa: Add Microchip KSZ switches binding
dsa: add maintainer of Microchip KSZ switches
Documentation/devicetree/bindings/net/dsa/ksz.txt | 72 +
MAINTAINERS | 10 +
drivers/net/dsa/Kconfig | 2 +
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/microchip/Kconfig | 12 +
drivers/net/dsa/microchip/Makefile | 2 +
drivers/net/dsa/microchip/ksz_9477_reg.h | 1676 +++++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.c | 1280 ++++++++++++++++
drivers/net/dsa/microchip/ksz_priv.h | 210 +++
drivers/net/dsa/microchip/ksz_spi.c | 215 +++
drivers/net/phy/micrel.c | 11 +
include/linux/micrel_phy.h | 2 +
include/linux/platform_data/microchip-ksz.h | 29 +
include/net/dsa.h | 1 +
net/dsa/Kconfig | 3 +
net/dsa/Makefile | 1 +
net/dsa/dsa.c | 3 +
net/dsa/dsa_priv.h | 3 +
net/dsa/tag_ksz.c | 101 ++
19 files changed, 3634 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/ksz.txt
create mode 100644 drivers/net/dsa/microchip/Kconfig
create mode 100644 drivers/net/dsa/microchip/Makefile
create mode 100644 drivers/net/dsa/microchip/ksz_9477_reg.h
create mode 100644 drivers/net/dsa/microchip/ksz_common.c
create mode 100644 drivers/net/dsa/microchip/ksz_priv.h
create mode 100644 drivers/net/dsa/microchip/ksz_spi.c
create mode 100644 include/linux/platform_data/microchip-ksz.h
create mode 100644 net/dsa/tag_ksz.c
--
2.7.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox