* [PATCH 1/8] batman-adv: Start new development cycle
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/main.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 057a28a9fe88..8da3c9336111 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -25,7 +25,7 @@
#define BATADV_DRIVER_DEVICE "batman-adv"
#ifndef BATADV_SOURCE_VERSION
-#define BATADV_SOURCE_VERSION "2018.1"
+#define BATADV_SOURCE_VERSION "2018.2"
#endif
/* B.A.T.M.A.N. parameters */
--
2.11.0
^ permalink raw reply related
* [PATCH 4/8] batman-adv: Avoid bool in structures
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Using the bool type for structure member is considered inappropriate [1]
for the kernel. Its size is not well defined (but usually 1 byte but maybe
also 4 byte).
[1] https://lkml.org/lkml/2017/11/21/384
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/types.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 0174f79e955a..3b3fc146b30d 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1160,13 +1160,13 @@ struct batadv_priv_dat {
*/
struct batadv_mcast_querier_state {
/** @exists: whether a querier exists in the mesh */
- bool exists;
+ unsigned char exists:1;
/**
* @shadowing: if a querier exists, whether it is potentially shadowing
* multicast listeners (i.e. querier is behind our own bridge segment)
*/
- bool shadowing;
+ unsigned char shadowing:1;
};
/**
@@ -1207,10 +1207,10 @@ struct batadv_priv_mcast {
u8 flags;
/** @enabled: whether the multicast tvlv is currently enabled */
- bool enabled;
+ unsigned char enabled:1;
/** @bridged: whether the soft interface has a bridge on top */
- bool bridged;
+ unsigned char bridged:1;
/**
* @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP
@@ -1389,7 +1389,7 @@ struct batadv_tp_vars {
atomic_t dup_acks;
/** @fast_recovery: true if in Fast Recovery mode */
- bool fast_recovery;
+ unsigned char fast_recovery:1;
/** @recover: last sent seqno when entering Fast Recovery */
u32 recover;
@@ -2046,10 +2046,10 @@ struct batadv_skb_cb {
* @decoded: Marks a skb as decoded, which is checked when searching for
* coding opportunities in network-coding.c
*/
- bool decoded;
+ unsigned char decoded:1;
/** @num_bcasts: Counter for broadcast packet retransmissions */
- unsigned int num_bcasts;
+ unsigned char num_bcasts;
};
/**
--
2.11.0
^ permalink raw reply related
* [PATCH 3/8] batman-adv: Avoid old nodes disabling multicast optimizations completely
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Linus Lüssing, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Linus Lüssing <linus.luessing@c0d3.blue>
Instead of disabling multicast optimizations mesh-wide once a node with
no multicast optimizations capabilities joins the mesh, do the
following:
Just insert such nodes into the WANT_ALL_IPV4/IPV6 lists. This is
sufficient to avoid multicast packet loss to such unsupportive nodes.
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/multicast.c | 29 ++++++-----------------------
net/batman-adv/soft-interface.c | 1 -
net/batman-adv/types.h | 3 ---
3 files changed, 6 insertions(+), 27 deletions(-)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index a11d3d89f012..36fd7b06c7cc 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -815,9 +815,6 @@ static int batadv_mcast_forw_mode_check(struct batadv_priv *bat_priv,
if (!atomic_read(&bat_priv->multicast_mode))
return -EINVAL;
- if (atomic_read(&bat_priv->mcast.num_disabled))
- return -EINVAL;
-
switch (ntohs(ethhdr->h_proto)) {
case ETH_P_IP:
return batadv_mcast_forw_mode_check_ipv4(bat_priv, skb,
@@ -1183,33 +1180,23 @@ static void batadv_mcast_tvlv_ogm_handler(struct batadv_priv *bat_priv,
{
bool orig_mcast_enabled = !(flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND);
u8 mcast_flags = BATADV_NO_FLAGS;
- bool orig_initialized;
if (orig_mcast_enabled && tvlv_value &&
tvlv_value_len >= sizeof(mcast_flags))
mcast_flags = *(u8 *)tvlv_value;
+ if (!orig_mcast_enabled) {
+ mcast_flags |= BATADV_MCAST_WANT_ALL_IPV4;
+ mcast_flags |= BATADV_MCAST_WANT_ALL_IPV6;
+ }
+
spin_lock_bh(&orig->mcast_handler_lock);
- orig_initialized = test_bit(BATADV_ORIG_CAPA_HAS_MCAST,
- &orig->capa_initialized);
- /* If mcast support is turned on decrease the disabled mcast node
- * counter only if we had increased it for this node before. If this
- * is a completely new orig_node no need to decrease the counter.
- */
if (orig_mcast_enabled &&
!test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities)) {
- if (orig_initialized)
- atomic_dec(&bat_priv->mcast.num_disabled);
set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities);
- /* If mcast support is being switched off or if this is an initial
- * OGM without mcast support then increase the disabled mcast
- * node counter.
- */
} else if (!orig_mcast_enabled &&
- (test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) ||
- !orig_initialized)) {
- atomic_inc(&bat_priv->mcast.num_disabled);
+ test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities)) {
clear_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities);
}
@@ -1595,10 +1582,6 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
spin_lock_bh(&orig->mcast_handler_lock);
- if (!test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) &&
- test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized))
- atomic_dec(&bat_priv->mcast.num_disabled);
-
batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
batadv_mcast_want_ipv4_update(bat_priv, orig, BATADV_NO_FLAGS);
batadv_mcast_want_ipv6_update(bat_priv, orig, BATADV_NO_FLAGS);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index edeffcb9f3a2..67065e35de51 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -796,7 +796,6 @@ static int batadv_softif_init_late(struct net_device *dev)
bat_priv->mcast.querier_ipv6.shadowing = false;
bat_priv->mcast.flags = BATADV_NO_FLAGS;
atomic_set(&bat_priv->multicast_mode, 1);
- atomic_set(&bat_priv->mcast.num_disabled, 0);
atomic_set(&bat_priv->mcast.num_want_all_unsnoopables, 0);
atomic_set(&bat_priv->mcast.num_want_all_ipv4, 0);
atomic_set(&bat_priv->mcast.num_want_all_ipv6, 0);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 476b052ad982..0174f79e955a 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1212,9 +1212,6 @@ struct batadv_priv_mcast {
/** @bridged: whether the soft interface has a bridge on top */
bool bridged;
- /** @num_disabled: number of nodes that have no mcast tvlv */
- atomic_t num_disabled;
-
/**
* @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP
* traffic
--
2.11.0
^ permalink raw reply related
* [PATCH 2/8] batman-adv: Disable CONFIG_BATMAN_ADV_DEBUGFS by default
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
All tools which were known to the batman-adv development team are
supporting the batman-adv netlink interface since a while. Also debugfs is
not supported for batman-adv interfaces in any non-default netns. Thus
disabling CONFIG_BATMAN_ADV_DEBUGFS by default should not cause problems on
most systems. It is still possible to enable it in case it is still
required in a specific setup.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig
index e4e2e02b7380..bee034a95208 100644
--- a/net/batman-adv/Kconfig
+++ b/net/batman-adv/Kconfig
@@ -94,13 +94,13 @@ config BATMAN_ADV_DEBUGFS
bool "batman-adv debugfs entries"
depends on BATMAN_ADV
depends on DEBUG_FS
- default y
+ default n
help
Enable this to export routing related debug tables via debugfs.
The information for each soft-interface and used hard-interface can be
found under batman_adv/
- If unsure, say Y.
+ If unsure, say N.
config BATMAN_ADV_DEBUG
bool "B.A.T.M.A.N. debugging"
--
2.11.0
^ permalink raw reply related
* [PATCH 5/8] batman-adv: Remove unused dentry without DEBUGFS
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
The debug_dir variable in the main structures is only accessed when
CONFIG_BATMAN_ADV_DEBUGFS is enabled. It can be dropped to potentially save
some bytes of memory.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/types.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 3b3fc146b30d..360357f83f20 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -215,10 +215,12 @@ struct batadv_hard_iface {
struct batadv_hard_iface_bat_v bat_v;
#endif
+#ifdef CONFIG_BATMAN_ADV_DEBUGFS
/**
* @debug_dir: dentry for nc subdir in batman-adv directory in debugfs
*/
struct dentry *debug_dir;
+#endif
/**
* @neigh_list: list of unique single hop neighbors via this interface
@@ -1242,10 +1244,12 @@ struct batadv_priv_nc {
/** @work: work queue callback item for cleanup */
struct delayed_work work;
+#ifdef CONFIG_BATMAN_ADV_DEBUGFS
/**
* @debug_dir: dentry for nc subdir in batman-adv directory in debugfs
*/
struct dentry *debug_dir;
+#endif
/**
* @min_tq: only consider neighbors for encoding if neigh_tq > min_tq
@@ -1598,8 +1602,10 @@ struct batadv_priv {
/** @mesh_obj: kobject for sysfs mesh subdirectory */
struct kobject *mesh_obj;
+#ifdef CONFIG_BATMAN_ADV_DEBUGFS
/** @debug_dir: dentry for debugfs batman-adv subdirectory */
struct dentry *debug_dir;
+#endif
/** @forw_bat_list: list of aggregated OGMs that will be forwarded */
struct hlist_head forw_bat_list;
--
2.11.0
^ permalink raw reply related
* [PATCH 7/8] batman-adv: disable ethtool link speed detection when auto negotiation off
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Marek Lindner <mareklindner@neomailbox.ch>
Virtual interface drivers such as tun / tap interfaces, VLAN, etc tend
to initialize the interface throughput with some value for the sake of
having a throughput number to export via ethtool. This exported
throughput leaves batman-adv to conclude the interface throughput is
genuine (reflecting reality), thus no measurements are necessary.
Based on the observation that those interface types also tend to set
the link auto-negotiation to 'off', batman-adv shall check this
setting to differentiate between genuine link throughput information
and placeholders installed by virtual interfaces.
The "default throughput" setting exported via sysfs still allows to
configure the batman-adv throughput for the interface, thus disabling
the measurements.
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Acked-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v_elp.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 28687493599f..71c20c1d4002 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -127,7 +127,20 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
rtnl_lock();
ret = __ethtool_get_link_ksettings(hard_iface->net_dev, &link_settings);
rtnl_unlock();
- if (ret == 0) {
+
+ /* Virtual interface drivers such as tun / tap interfaces, VLAN, etc
+ * tend to initialize the interface throughput with some value for the
+ * sake of having a throughput number to export via ethtool. This
+ * exported throughput leaves batman-adv to conclude the interface
+ * throughput is genuine (reflecting reality), thus no measurements
+ * are necessary.
+ *
+ * Based on the observation that those interface types also tend to set
+ * the link auto-negotiation to 'off', batman-adv shall check this
+ * setting to differentiate between genuine link throughput information
+ * and placeholders installed by virtual interfaces.
+ */
+ if (ret == 0 && link_settings.base.autoneg == AUTONEG_ENABLE) {
/* link characteristics might change over time */
if (link_settings.base.duplex == DUPLEX_FULL)
hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
--
2.11.0
^ permalink raw reply related
* [PATCH 6/8] batman-adv: fix batadv_interface_tx()'s return type
From: Simon Wunderlich @ 2018-05-24 12:02 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Luc Van Oostenryck, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, but the implementation in this
driver returns an 'int'.
Fix this by returning 'netdev_tx_t' in this driver too.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
[sven@narfation.org: fixed alignment]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/soft-interface.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 67065e35de51..1485263a348b 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -188,8 +188,8 @@ static void batadv_interface_set_rx_mode(struct net_device *dev)
{
}
-static int batadv_interface_tx(struct sk_buff *skb,
- struct net_device *soft_iface)
+static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
+ struct net_device *soft_iface)
{
struct ethhdr *ethhdr;
struct batadv_priv *bat_priv = netdev_priv(soft_iface);
--
2.11.0
^ permalink raw reply related
* [PATCH 8/8] batman-adv: enable B.A.T.M.A.N. V compilation by default
From: Simon Wunderlich @ 2018-05-24 12:03 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20180524120300.15829-1-sw@simonwunderlich.de>
From: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Acked-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig
index bee034a95208..de8034d80623 100644
--- a/net/batman-adv/Kconfig
+++ b/net/batman-adv/Kconfig
@@ -35,7 +35,7 @@ config BATMAN_ADV
config BATMAN_ADV_BATMAN_V
bool "B.A.T.M.A.N. V protocol (experimental)"
depends on BATMAN_ADV && !(CFG80211=m && BATMAN_ADV=y)
- default n
+ default y
help
This option enables the B.A.T.M.A.N. V protocol, the successor
of the currently used B.A.T.M.A.N. IV protocol. The main
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 8/8] batman-adv: enable B.A.T.M.A.N. V compilation by default
From: Joe Perches @ 2018-05-24 12:09 UTC (permalink / raw)
To: Simon Wunderlich, davem
Cc: netdev, b.a.t.m.a.n, Marek Lindner, Sven Eckelmann
In-Reply-To: <20180524120300.15829-9-sw@simonwunderlich.de>
On Thu, 2018-05-24 at 14:03 +0200, Simon Wunderlich wrote:
> From: Marek Lindner <mareklindner@neomailbox.ch>
>
> Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
> Acked-by: Antonio Quartulli <a@unstable.cc>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> ---
> net/batman-adv/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig
> index bee034a95208..de8034d80623 100644
> --- a/net/batman-adv/Kconfig
> +++ b/net/batman-adv/Kconfig
> @@ -35,7 +35,7 @@ config BATMAN_ADV
> config BATMAN_ADV_BATMAN_V
> bool "B.A.T.M.A.N. V protocol (experimental)"
> depends on BATMAN_ADV && !(CFG80211=m && BATMAN_ADV=y)
> - default n
> + default y
Why should anything experimental be enabled by default?
> help
> This option enables the B.A.T.M.A.N. V protocol, the successor
> of the currently used B.A.T.M.A.N. IV protocol. The main
^ permalink raw reply
* Re: [PATCH net-next] net:sched: add action inheritdsfield to skbmod
From: Jamal Hadi Salim @ 2018-05-24 12:11 UTC (permalink / raw)
To: Cong Wang, Fu, Qiaobin
Cc: davem@davemloft.net, netdev@vger.kernel.org, Michel Machado
In-Reply-To: <CAM_iQpWJ0=w5A5+onbF-Ngo+emgHm-JMpyJdWONt2NWRu9Kk2w@mail.gmail.com>
On 23/05/18 07:01 PM, Cong Wang wrote:
> On Thu, May 17, 2018 at 12:33 PM, Fu, Qiaobin <qiaobinf@bu.edu> wrote:
> Hmm, but skbedit seems better than skbmod for this job,
> given:
>
> 1) It already modifies skb->priority, although with a given value
>
> 2) skbmod doesn't change skb metadata, it only changes payload
>
> I am _not_ saying there is strict rule for what skbmod can or can't
> change, it calls itself "data modifier", so I am saying we probably
> need to follow this existing practice.
>
I am indifferent - you can move it to skbedit.
Note: I have patches which i will send out at some point when
I get the chance on pedit for implementing the concept of
"copy data to metadata" and "copy metadata to data"
for pedit it made a lot of sense to add the feature there.
In this case skbedit makes more sense.
cheers,
jamal
^ permalink raw reply
* [PATCH net-next v2] cxgb4/cxgb4vf: link management changes for new SFP
From: Ganesh Goudar @ 2018-05-24 12:19 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, leedom, Ganesh Goudar
newer SFPs like SFP28 and QSFP28 Transceiver Modules present
several new possibilities which we haven't faced before. Fix the
assumptions in the code reflecting the more limited capabilities
of previous Transceiver Module systems
Original work by Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
V2: Was not getting applied on net-next, respining on net-next
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c | 22 +++++-----
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 34 ++++++++++++++--
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c | 47 +++++++++++++++++++++-
3 files changed, 85 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
index 59d04d7..f7eef93 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
@@ -800,24 +800,20 @@ static int set_link_ksettings(struct net_device *dev,
if (base->duplex != DUPLEX_FULL)
return -EINVAL;
- if (!(lc->pcaps & FW_PORT_CAP32_ANEG)) {
- /* PHY offers a single speed. See if that's what's
- * being requested.
- */
- if (base->autoneg == AUTONEG_DISABLE &&
- (lc->pcaps & speed_to_fw_caps(base->speed)))
- return 0;
- return -EINVAL;
- }
-
old_lc = *lc;
- if (base->autoneg == AUTONEG_DISABLE) {
+ if (!(lc->pcaps & FW_PORT_CAP32_ANEG) ||
+ base->autoneg == AUTONEG_DISABLE) {
fw_caps = speed_to_fw_caps(base->speed);
- if (!(lc->pcaps & fw_caps))
+ /* Must only specify a single speed which must be supported
+ * as part of the Physical Port Capabilities.
+ */
+ if ((fw_caps & (fw_caps - 1)) != 0 ||
+ !(lc->pcaps & fw_caps))
return -EINVAL;
+
lc->speed_caps = fw_caps;
- lc->acaps = 0;
+ lc->acaps = fw_caps;
} else {
fw_caps =
lmm_to_fw_caps(link_ksettings->link_modes.advertising);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 704f696..962384c 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -4066,6 +4066,7 @@ int t4_link_l1cfg_core(struct adapter *adapter, unsigned int mbox,
fw_port_cap32_t fw_fc, cc_fec, fw_fec, rcap;
struct fw_port_cmd cmd;
unsigned int fw_mdi;
+ int ret;
fw_mdi = (FW_PORT_CAP32_MDI_V(FW_PORT_CAP32_MDI_AUTO) & lc->pcaps);
/* Convert driver coding of Pause Frame Flow Control settings into the
@@ -4100,6 +4101,13 @@ int t4_link_l1cfg_core(struct adapter *adapter, unsigned int mbox,
rcap = lc->acaps | fw_fc | fw_fec | fw_mdi;
}
+ if (rcap & ~lc->pcaps) {
+ dev_err(adapter->pdev_dev,
+ "Requested Port Capabilities %#x exceed Physical Port Capabilities %#x\n",
+ rcap, lc->pcaps);
+ return -EINVAL;
+ }
+
/* And send that on to the Firmware ...
*/
memset(&cmd, 0, sizeof(cmd));
@@ -4110,13 +4118,21 @@ int t4_link_l1cfg_core(struct adapter *adapter, unsigned int mbox,
cpu_to_be32(FW_PORT_CMD_ACTION_V(fw_caps == FW_CAPS16
? FW_PORT_ACTION_L1_CFG
: FW_PORT_ACTION_L1_CFG32) |
- FW_LEN16(cmd));
+ FW_LEN16(cmd));
if (fw_caps == FW_CAPS16)
cmd.u.l1cfg.rcap = cpu_to_be32(fwcaps32_to_caps16(rcap));
else
cmd.u.l1cfg32.rcap32 = cpu_to_be32(rcap);
- return t4_wr_mbox_meat_timeout(adapter, mbox, &cmd, sizeof(cmd), NULL,
- sleep_ok, timeout);
+
+ ret = t4_wr_mbox_meat_timeout(adapter, mbox, &cmd, sizeof(cmd), NULL,
+ sleep_ok, timeout);
+ if (ret) {
+ dev_err(adapter->pdev_dev,
+ "Requested Port Capabilities %#x rejected, error %d\n",
+ rcap, -ret);
+ return ret;
+ }
+ return ret;
}
/**
@@ -8395,7 +8411,9 @@ void t4_handle_get_port_info(struct port_info *pi, const __be64 *rpl)
lc->lpacaps = lpacaps;
lc->acaps = acaps & ADVERT_MASK;
- if (lc->acaps & FW_PORT_CAP32_ANEG) {
+ if (!(lc->acaps & FW_PORT_CAP32_ANEG)) {
+ lc->autoneg = AUTONEG_DISABLE;
+ } else if (lc->acaps & FW_PORT_CAP32_ANEG) {
lc->autoneg = AUTONEG_ENABLE;
} else {
/* When Autoneg is disabled, user needs to set
@@ -8600,6 +8618,13 @@ static void init_link_config(struct link_config *lc, fw_port_cap32_t pcaps,
lc->requested_fec = FEC_AUTO;
lc->fec = fwcap_to_cc_fec(lc->def_acaps);
+ /* If the Port is capable of Auto-Negtotiation, initialize it as
+ * "enabled" and copy over all of the Physical Port Capabilities
+ * to the Advertised Port Capabilities. Otherwise mark it as
+ * Auto-Negotiate disabled and select the highest supported speed
+ * for the link. Note parallel structure in t4_link_l1cfg_core()
+ * and t4_handle_get_port_info().
+ */
if (lc->pcaps & FW_PORT_CAP32_ANEG) {
lc->acaps = lc->pcaps & ADVERT_MASK;
lc->autoneg = AUTONEG_ENABLE;
@@ -8607,6 +8632,7 @@ static void init_link_config(struct link_config *lc, fw_port_cap32_t pcaps,
} else {
lc->acaps = 0;
lc->autoneg = AUTONEG_DISABLE;
+ lc->speed_caps = fwcap_to_fwspeed(acaps);
}
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
index 3017f78..8786431 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
@@ -405,6 +405,36 @@ static unsigned int fwcap_to_speed(fw_port_cap32_t caps)
return 0;
}
+/**
+ * fwcap_to_fwspeed - return highest speed in Port Capabilities
+ * @acaps: advertised Port Capabilities
+ *
+ * Get the highest speed for the port from the advertised Port
+ * Capabilities. It will be either the highest speed from the list of
+ * speeds or whatever user has set using ethtool.
+ */
+static fw_port_cap32_t fwcap_to_fwspeed(fw_port_cap32_t acaps)
+{
+ #define TEST_SPEED_RETURN(__caps_speed) \
+ do { \
+ if (acaps & FW_PORT_CAP32_SPEED_##__caps_speed) \
+ return FW_PORT_CAP32_SPEED_##__caps_speed; \
+ } while (0)
+
+ TEST_SPEED_RETURN(400G);
+ TEST_SPEED_RETURN(200G);
+ TEST_SPEED_RETURN(100G);
+ TEST_SPEED_RETURN(50G);
+ TEST_SPEED_RETURN(40G);
+ TEST_SPEED_RETURN(25G);
+ TEST_SPEED_RETURN(10G);
+ TEST_SPEED_RETURN(1G);
+ TEST_SPEED_RETURN(100M);
+
+ #undef TEST_SPEED_RETURN
+ return 0;
+}
+
/*
* init_link_config - initialize a link's SW state
* @lc: structure holding the link state
@@ -431,6 +461,13 @@ static void init_link_config(struct link_config *lc,
lc->requested_fec = FEC_AUTO;
lc->fec = lc->auto_fec;
+ /* If the Port is capable of Auto-Negtotiation, initialize it as
+ * "enabled" and copy over all of the Physical Port Capabilities
+ * to the Advertised Port Capabilities. Otherwise mark it as
+ * Auto-Negotiate disabled and select the highest supported speed
+ * for the link. Note parallel structure in t4_link_l1cfg_core()
+ * and t4_handle_get_port_info().
+ */
if (lc->pcaps & FW_PORT_CAP32_ANEG) {
lc->acaps = acaps & ADVERT_MASK;
lc->autoneg = AUTONEG_ENABLE;
@@ -438,6 +475,7 @@ static void init_link_config(struct link_config *lc,
} else {
lc->acaps = 0;
lc->autoneg = AUTONEG_DISABLE;
+ lc->speed_caps = fwcap_to_fwspeed(acaps);
}
}
@@ -1955,7 +1993,14 @@ static void t4vf_handle_get_port_info(struct port_info *pi,
lc->lpacaps = lpacaps;
lc->acaps = acaps & ADVERT_MASK;
- if (lc->acaps & FW_PORT_CAP32_ANEG) {
+ /* If we're not physically capable of Auto-Negotiation, note
+ * this as Auto-Negotiation disabled. Otherwise, we track
+ * what Auto-Negotiation settings we have. Note parallel
+ * structure in init_link_config().
+ */
+ if (!(lc->pcaps & FW_PORT_CAP32_ANEG)) {
+ lc->autoneg = AUTONEG_DISABLE;
+ } else if (lc->acaps & FW_PORT_CAP32_ANEG) {
lc->autoneg = AUTONEG_ENABLE;
} else {
/* When Autoneg is disabled, user needs to set
--
2.1.0
^ permalink raw reply related
* [PATCH net-next] tcp: use data length instead of skb->len in tcp_probe
From: Yafang Shao @ 2018-05-24 12:48 UTC (permalink / raw)
To: davem, songliubraving; +Cc: netdev, linux-kernel, Yafang Shao
skb->len is meaningless to user.
data length could be more helpful, with which we can easily filter out
the packet without payload.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/trace/events/tcp.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index c1a5284..259b991 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -261,7 +261,7 @@
__entry->dport = ntohs(inet->inet_dport);
__entry->mark = skb->mark;
- __entry->length = skb->len;
+ __entry->length = skb->len - tcp_hdrlen(skb);
__entry->snd_nxt = tp->snd_nxt;
__entry->snd_una = tp->snd_una;
__entry->snd_cwnd = tp->snd_cwnd;
@@ -272,7 +272,7 @@
__entry->sock_cookie = sock_gen_cookie(sk);
),
- TP_printk("src=%pISpc dest=%pISpc mark=%#x length=%d snd_nxt=%#x snd_una=%#x snd_cwnd=%u ssthresh=%u snd_wnd=%u srtt=%u rcv_wnd=%u sock_cookie=%llx",
+ TP_printk("src=%pISpc dest=%pISpc mark=%#x data_len=%d snd_nxt=%#x snd_una=%#x snd_cwnd=%u ssthresh=%u snd_wnd=%u srtt=%u rcv_wnd=%u sock_cookie=%llx",
__entry->saddr, __entry->daddr, __entry->mark,
__entry->length, __entry->snd_nxt, __entry->snd_una,
__entry->snd_cwnd, __entry->ssthresh, __entry->snd_wnd,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 0/4] RFC CPSW switchdev mode
From: Andrew Lunn @ 2018-05-24 12:54 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Jiri Pirko, netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar,
ivecera, francois.ozog, yogeshs, spatton
In-Reply-To: <20180524084831.GA2759@apalos>
On Thu, May 24, 2018 at 11:48:31AM +0300, Ilias Apalodimas wrote:
> On Thu, May 24, 2018 at 10:05:28AM +0200, Jiri Pirko wrote:
> > Thu, May 24, 2018 at 08:56:20AM CEST, ilias.apalodimas@linaro.org wrote:
> > Any reason you need cpu port? We don't need it in mlxsw and also in dsa.
> Yes i've seen that on mlxsw/rocker drivers and i was reluctant adding one here.
> The reason is that TI wants this configured differently from customer facing
> ports. Apparently there are existing customers already using the "feature".
> So OR'ing and adding the cpu port on every operation (add/del vlans add
> ucast/mcast entries etc) was less favoured.
Hi Ilias
Nice to see this device moving away from its custom model and towards
the switchdev model.
Did you consider making a clean break from the existing code and write
a new driver. Let the existing customers using the existing
driver. Have the new switchdev driver fully conform to switchdev.
I don't like having this 'cpu' interface. As you say, it breaks the
switchhdev model. If we need to extend the switchdev model to support
some use case, lets do that. Please can you fully describe the use
cases, so we can discuss how to implement them cleanly within the
switchdev model.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v2 net] stmmac: strip vlan tag on reception only for 8021q tagged frames
From: Jose Abreu @ 2018-05-24 12:56 UTC (permalink / raw)
To: Elad Nachman, Jose Abreu, Florian Fainelli, David Miller
Cc: makita.toshiaki, netdev, peppe.cavallaro, alexandre.torgue
In-Reply-To: <cb8a0eaa-43c8-902c-ebd3-2aa90bf868b2@gmail.com>
On 23-05-2018 16:00, Elad Nachman wrote:
> Jose,
>
> I am not sure which drivers you have checked. I guess most non-networking embedded drivers never use 802.1AD
> so they stay broken unknowingly.
> Specifically, I have tested Intel e1000e based card which works correctly versus stmmac which works incorrectly.
>
> If you check netdev.c in e1000e then the vlan strip is conditional upon checking of 802.1Q bit in the descriptor,
> which does not happen in stmmac_main.c - the vlan stripping happens based on any tag header check.
> Once I applied my patch things started working - did not have to patch e1000e. The problem is that ip link allows to create 802.1ad devices which are not 0x8100 tagged but stmmac and probably other drivers handles the non 0x8100 tag incorrectly and the vlan slave discards the frame.
>
> Regarding the getting the tag from the descriptor - this is of course a possibility. My original patch handled stripping of all tags but then I was told here that I cannot strip 802.1ad tags without the NETIF_F_HW_VLAN_STAG_RX feature, which is probably correct regardless of the source of the vlan tag - skb or descriptor.
>
> I can also add the NETIF_F_HW_VLAN_STAG_RX feature plus the following original v1 patch:
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 2018-04-11 17:04:00.586057300 +0300
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 2018-04-11 17:05:33.601992400 +0300
> @@ -3293,17 +3293,19 @@ dma_map_err:
>
> static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
> {
> - struct ethhdr *ehdr;
> + struct vlan_ethhdr *veth;
> u16 vlanid;
> + __be16 vlan_proto;
>
> if ((dev->features & NETIF_F_HW_VLAN_CTAG_RX) ==
> NETIF_F_HW_VLAN_CTAG_RX &&
> !__vlan_get_tag(skb, &vlanid)) {
> /* pop the vlan tag */
> - ehdr = (struct ethhdr *)skb->data;
> - memmove(skb->data + VLAN_HLEN, ehdr, ETH_ALEN * 2);
> + veth = (struct vlan_ethhdr *)skb->data;
> + vlan_proto = veth->h_vlan_proto;
> + memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
> skb_pull(skb, VLAN_HLEN);
> - __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlanid);
> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlanid);
> }
> }
>
> There are three reasons why I prefer using this approach rather than the descriptor approach:
>
> A. It is inline with the original driver code.
> B. Using descriptor vlan will require to completely rewrite stmmac_rx_vlan and will result in at least 2-3 times line counts in the patch.
> C. I have no idea if first silicon implementations of DWMAC IP had some kind of HW bug (for example AXI clock glitch) which caused sampling of the VLAN tag in the descriptors to be unstable, and since I have no access to such hardware for regression I risk breaking stmmac for such old SOCs in case they decide to jump kernel versions to the latest.
Your approach seems okay by me. Please submit a patch with this
to netdev for proper review.
Thanks and Best Regards,
Jose Miguel Abreu
>
> Thanks,
>
> Elad.
>
> On 22/05/18 11:56, Jose Abreu wrote:
>> On 21-05-2018 17:42, Florian Fainelli wrote:
>>> On 05/21/2018 08:48 AM, David Miller wrote:
>>>> From: David Miller <davem@davemloft.net>
>>>> Date: Thu, 17 May 2018 12:43:56 -0400 (EDT)
>>>>
>>>>> Giuseppe and Alexandre, please review this patch.
>>>> If nobody thinks this patch is important enough to actually
>>>> review, I'm tossing it.
>>>>
>>>> Sorry.
>>>>
>>> How about looping in Jose?
>> Thanks for the cc Florian!
>>
>> Elad,
>>
>> Looking at this patch I have a couple of questions and suggestions:
>>
>> I see that most drivers use this pattern so, are they all broken?
>> or is this a special case?
>>
>> You can also get the inner/outer vlan tag by reading desc0 of
>> receive descriptor. Which can make this completely agnostic of
>> VLAN tag.
>>
>> Thanks and Best Regards,
>> Jose Miguel Abreu
>>
^ permalink raw reply
* [PATCH net-next] cxgb4: clean up init_one
From: Ganesh Goudar @ 2018-05-24 13:02 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, Ganesh Goudar, Casey Leedom
clean up init_one and use chip_ver consistently throughout
init_one() for chip version.
Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 47 +++++++++++++----------
drivers/net/ethernet/chelsio/cxgb4/t4_chip_type.h | 2 +
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 513e1d3..c1790785 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5236,14 +5236,11 @@ static void free_some_resources(struct adapter *adapter)
NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
#define SEGMENT_SIZE 128
-static int get_chip_type(struct pci_dev *pdev, u32 pl_rev)
+static int t4_get_chip_type(struct adapter *adap, int ver)
{
- u16 device_id;
-
- /* Retrieve adapter's device ID */
- pci_read_config_word(pdev, PCI_DEVICE_ID, &device_id);
+ u32 pl_rev = REV_G(t4_read_reg(adap, PL_REV_A));
- switch (device_id >> 12) {
+ switch (ver) {
case CHELSIO_T4:
return CHELSIO_CHIP_CODE(CHELSIO_T4, pl_rev);
case CHELSIO_T5:
@@ -5251,8 +5248,7 @@ static int get_chip_type(struct pci_dev *pdev, u32 pl_rev)
case CHELSIO_T6:
return CHELSIO_CHIP_CODE(CHELSIO_T6, pl_rev);
default:
- dev_err(&pdev->dev, "Device %d is not supported\n",
- device_id);
+ break;
}
return -EINVAL;
}
@@ -5422,15 +5418,18 @@ static int cxgb4_iov_configure(struct pci_dev *pdev, int num_vfs)
static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
- int func, i, err, s_qpp, qpp, num_seg;
+ struct net_device *netdev;
+ struct adapter *adapter;
+ static int adap_idx = 1;
+ int s_qpp, qpp, num_seg;
struct port_info *pi;
bool highdma = false;
- struct adapter *adapter = NULL;
- struct net_device *netdev;
- void __iomem *regs;
- u32 whoami, pl_rev;
enum chip_type chip;
- static int adap_idx = 1;
+ void __iomem *regs;
+ int func, chip_ver;
+ u16 device_id;
+ int i, err;
+ u32 whoami;
printk_once(KERN_INFO "%s - version %s\n", DRV_DESC, DRV_VERSION);
@@ -5466,11 +5465,17 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
goto out_free_adapter;
/* We control everything through one PF */
- whoami = readl(regs + PL_WHOAMI_A);
- pl_rev = REV_G(readl(regs + PL_REV_A));
- chip = get_chip_type(pdev, pl_rev);
- func = CHELSIO_CHIP_VERSION(chip) <= CHELSIO_T5 ?
- SOURCEPF_G(whoami) : T6_SOURCEPF_G(whoami);
+ whoami = t4_read_reg(adapter, PL_WHOAMI_A);
+ pci_read_config_word(pdev, PCI_DEVICE_ID, &device_id);
+ chip = t4_get_chip_type(adapter, CHELSIO_PCI_ID_VER(device_id));
+ if (chip < 0) {
+ dev_err(&pdev->dev, "Device %d is not supported\n", device_id);
+ err = chip;
+ goto out_free_adapter;
+ }
+ chip_ver = CHELSIO_CHIP_VERSION(chip);
+ func = chip_ver <= CHELSIO_T5 ?
+ SOURCEPF_G(whoami) : T6_SOURCEPF_G(whoami);
adapter->pdev = pdev;
adapter->pdev_dev = &pdev->dev;
@@ -5636,7 +5641,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_TC;
- if (CHELSIO_CHIP_VERSION(chip) > CHELSIO_T5) {
+ if (chip_ver > CHELSIO_T5) {
netdev->hw_enc_features |= NETIF_F_IP_CSUM |
NETIF_F_IPV6_CSUM |
NETIF_F_RXCSUM |
@@ -5716,7 +5721,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dev_warn(&pdev->dev, "could not allocate MPS Encap entries, continuing\n");
#if IS_ENABLED(CONFIG_IPV6)
- if ((CHELSIO_CHIP_VERSION(adapter->params.chip) <= CHELSIO_T5) &&
+ if (chip_ver <= CHELSIO_T5 &&
(!(t4_read_reg(adapter, LE_DB_CONFIG_A) & ASLIPCOMPEN_F))) {
/* CLIP functionality is not present in hardware,
* hence disable all offload features
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_chip_type.h b/drivers/net/ethernet/chelsio/cxgb4/t4_chip_type.h
index 54b7181..721c775 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_chip_type.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_chip_type.h
@@ -34,6 +34,8 @@
#ifndef __T4_CHIP_TYPE_H__
#define __T4_CHIP_TYPE_H__
+#define CHELSIO_PCI_ID_VER(__DeviceID) ((__DeviceID) >> 12)
+
#define CHELSIO_T4 0x4
#define CHELSIO_T5 0x5
#define CHELSIO_T6 0x6
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 4/4] cpsw: add switchdev support
From: Andrew Lunn @ 2018-05-24 13:12 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar, jiri,
ivecera, francois.ozog, yogeshs, spatton
In-Reply-To: <1527144984-31236-5-git-send-email-ilias.apalodimas@linaro.org>
> @@ -2626,7 +2750,11 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
> data->mac_control = prop;
>
> if (of_property_read_bool(node, "dual_emac"))
> - data->dual_emac = 1;
> + data->switch_mode = CPSW_DUAL_EMAC;
> +
> + /* switchdev overrides DTS */
> + if (IS_ENABLED(CONFIG_TI_CPSW_SWITCHDEV))
> + data->switch_mode = CPSW_SWITCHDEV;
Device tree is supposed to describe the hardware. Using that hardware
in different ways is not something you should describe in DT.
There are also a lot of IS_ENABLED() here, which i don't like. It is a
lot better than #ifdef, but we should try to do better. It would be
good to split this cleanly into three parts. A generic library, which
does not care about DUAL_MAC or SWITCHDEV. A driver which implements
legacy DUAL MAC etc. And a driver which implements SWITCHDEV. We can
then give this new switchdev driver a different compatible. It i still
encoding in device tree how to use the hardware, but it is more
implicit, rather than explicit.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] Add support for QCA8334 switch
From: Andrew Lunn @ 2018-05-24 13:17 UTC (permalink / raw)
To: Michal Vokáč
Cc: Florian Fainelli, netdev, linux-kernel, devicetree,
vivien.didelot, mark.rutland, robh+dt, davem, michal.vokac
In-Reply-To: <f8bf325c-77a5-d44b-34c3-93ea4b11fd84@gmail.com>
> Thank you too Florian. And also big thank to you Andrew. You helped me
> a lot to debug the RGMII issue. I have been stuck at that for more than
> a month and would not resolve it without your help.
We are here to help. I also got stuck figuring out RGMII issues, so i
know what it feels like...
> As I have done this in a process of upgrading our BSP to a more recent
> kernel, and hopefully mainline, I now need to move on to other parts of
> the board.
Do you think you can mainline the board? It would be nice to have an
in kernel board using this switch. If you post the device tree
patches, please Cc: me.
Andrew
^ permalink raw reply
* Re: [PATCH 1/6] ravb: remove custom .nway_reset from ethtool ops
From: Andrew Lunn @ 2018-05-24 13:22 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: David S. Miller, Sergei Shtylyov, netdev, linux-renesas-soc
In-Reply-To: <1527160318-10958-2-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, May 24, 2018 at 02:11:53PM +0300, Vladimir Zapolskiy wrote:
> The change fixes a sleep in atomic context issue, which can be
> always triggered by running 'ethtool -r' command, because
> phy_start_aneg() protects phydev fields by a mutex.
>
> Another note is that the change implicitly replaces phy_start_aneg()
> with a newer phy_restart_aneg().
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/net/ethernet/renesas/ravb_main.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 68f122140966..4a043eb0e2aa 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1150,21 +1150,6 @@ static int ravb_set_link_ksettings(struct net_device *ndev,
> return error;
> }
>
> -static int ravb_nway_reset(struct net_device *ndev)
> -{
> - struct ravb_private *priv = netdev_priv(ndev);
> - int error = -ENODEV;
> - unsigned long flags;
> -
> - if (ndev->phydev) {
> - spin_lock_irqsave(&priv->lock, flags);
> - error = phy_start_aneg(ndev->phydev);
> - spin_unlock_irqrestore(&priv->lock, flags);
> - }
Eck! phylib assumes thread context and takes a mutex while calling
into the PHY driver.
It would be good to add some sort of fixes: tag. Maybe for the commit
that added the generic nway_reset? That would at least cover some
stable kernels.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 3/6] ravb: remove custom .set_link_ksettings from ethtool ops
From: Andrew Lunn @ 2018-05-24 13:29 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: David S. Miller, Sergei Shtylyov, netdev, linux-renesas-soc
In-Reply-To: <1527160318-10958-4-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, May 24, 2018 at 02:11:55PM +0300, Vladimir Zapolskiy wrote:
> The change replaces a custom implementation of .set_link_ksettings
> callback with a shared phy_ethtool_set_link_ksettings(), this fixes
> sleep in atomic context bug, which is encountered every time when link
> settings are changed by ethtool.
>
> Now duplex mode setting is enforced in ravb_adjust_link() only, also
> now TX/RX is disabled when link is put down or modifications to E-MAC
> registers ECMR and GECMR are expected for both cases of checked and
> ignored link status pin state from E-MAC interrupt handler.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/net/ethernet/renesas/ravb_main.c | 58 +++++++++-----------------------
> 1 file changed, 15 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 3d91caa44176..0d811c02ff34 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -980,6 +980,13 @@ static void ravb_adjust_link(struct net_device *ndev)
> struct ravb_private *priv = netdev_priv(ndev);
> struct phy_device *phydev = ndev->phydev;
> bool new_state = false;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->lock, flags);
Hi Vladimir
It is pretty unusual to see an adjust_link callback take a lock. Is it
clearly defined what it is protecting?
Andrew
^ permalink raw reply
* Re: [PATCH 4/6] sh_eth: remove custom .nway_reset from ethtool ops
From: Andrew Lunn @ 2018-05-24 13:30 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: David S. Miller, Sergei Shtylyov, netdev, linux-renesas-soc
In-Reply-To: <1527160318-10958-5-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, May 24, 2018 at 02:11:56PM +0300, Vladimir Zapolskiy wrote:
> The change fixes a sleep in atomic context issue, which can be
> always triggered by running 'ethtool -r' command, because
> phy_start_aneg() protects phydev fields by a mutex.
>
> Another note is that the change implicitly replaces phy_start_aneg()
> with a newer phy_restart_aneg().
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 5/6] sh_eth: remove custom .get_link_ksettings from ethtool ops
From: Andrew Lunn @ 2018-05-24 13:31 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: David S. Miller, Sergei Shtylyov, netdev, linux-renesas-soc
In-Reply-To: <1527160484-11087-1-git-send-email-vladimir_zapolskiy@mentor.com>
On Thu, May 24, 2018 at 02:14:43PM +0300, Vladimir Zapolskiy wrote:
> The change replaces a custom implementation of .get_link_ksettings
> callback with a shared phy_ethtool_get_link_ksettings(), note that
> &priv->lock wrapping is not needed, because the lock does not
> serialize access to phydev fields.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 4/4] cpsw: add switchdev support
From: Ilias Apalodimas @ 2018-05-24 13:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar, jiri,
ivecera, francois.ozog, yogeshs, spatton
In-Reply-To: <20180524131229.GC24557@lunn.ch>
On Thu, May 24, 2018 at 03:12:29PM +0200, Andrew Lunn wrote:
> Device tree is supposed to describe the hardware. Using that hardware
> in different ways is not something you should describe in DT.
>
The new switchdev mode is applied with a .config option in the kernel. What you
see is pre-existing code, so i am not sure if i should change it in this
patchset. Your point is valid though and we are on the same page.
> There are also a lot of IS_ENABLED() here, which i don't like. It is a
> lot better than #ifdef, but we should try to do better.
I don't like it either i just tried to clean up code in "hot path" with ifdefs.
In theory this should replace "switch mode" in the near future so the ifdefs
will go away
> It would be
> good to split this cleanly into three parts. A generic library, which
> does not care about DUAL_MAC or SWITCHDEV. A driver which implements
> legacy DUAL MAC etc. And a driver which implements SWITCHDEV. We can
> then give this new switchdev driver a different compatible. It i still
> encoding in device tree how to use the hardware, but it is more
> implicit, rather than explicit.
Good idea, i'll sent the next version like that
Thanks,
Ilias
^ permalink raw reply
* Re: [PATCH net-next] bpfilter: fix build dependency
From: David Miller @ 2018-05-24 13:33 UTC (permalink / raw)
To: ast; +Cc: daniel, jakub.kicinski, netdev, kernel-team
In-Reply-To: <20180524042905.3605566-1-ast@kernel.org>
From: Alexei Starovoitov <ast@kernel.org>
Date: Wed, 23 May 2018 21:29:05 -0700
> BPFILTER could have been enabled without INET causing this build error:
> ERROR: "bpfilter_process_sockopt" [net/bpfilter/bpfilter.ko] undefined!
>
> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bpfilter: don't pass O_CREAT when opening console for debug
From: David Miller @ 2018-05-24 13:37 UTC (permalink / raw)
To: jakub.kicinski; +Cc: alexei.starovoitov, netdev, oss-drivers
In-Reply-To: <20180524074119.32132-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 24 May 2018 00:41:19 -0700
> Passing O_CREAT (00000100) to open means we should also pass file
> mode as the third parameter. Creating /dev/console as a regular
> file may not be helpful anyway, so simply drop the flag when
> opening debug_fd.
>
> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied, thanks Jakub.
^ permalink raw reply
* Re: [PATCH 0/4] RFC CPSW switchdev mode
From: Ivan Vecera @ 2018-05-24 13:44 UTC (permalink / raw)
To: Andrew Lunn, Ilias Apalodimas
Cc: Jiri Pirko, netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar,
francois.ozog, yogeshs, spatton
In-Reply-To: <20180524125431.GB24557@lunn.ch>
On 24.5.2018 14:54, Andrew Lunn wrote:
> On Thu, May 24, 2018 at 11:48:31AM +0300, Ilias Apalodimas wrote:
>> On Thu, May 24, 2018 at 10:05:28AM +0200, Jiri Pirko wrote:
>>> Thu, May 24, 2018 at 08:56:20AM CEST, ilias.apalodimas@linaro.org wrote:
>>> Any reason you need cpu port? We don't need it in mlxsw and also in dsa.
>> Yes i've seen that on mlxsw/rocker drivers and i was reluctant adding one here.
>> The reason is that TI wants this configured differently from customer facing
>> ports. Apparently there are existing customers already using the "feature".
>> So OR'ing and adding the cpu port on every operation (add/del vlans add
>> ucast/mcast entries etc) was less favoured.
>
> Hi Ilias
>
> Nice to see this device moving away from its custom model and towards
> the switchdev model.
+1
> Did you consider making a clean break from the existing code and write
> a new driver. Let the existing customers using the existing
> driver. Have the new switchdev driver fully conform to switchdev.
I would also prefer fresh new driver. The existing one can be marked as
'bugfix-only' and later pertinently deprecated/removed.
>
> I don't like having this 'cpu' interface. As you say, it breaks the
> switchhdev model. If we need to extend the switchdev model to support
> some use case, lets do that. Please can you fully describe the use
> cases, so we can discuss how to implement them cleanly within the
> switchdev model.
+1
Ivan
^ 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