* [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
@ 2021-01-15 12:52 Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tobias Waldekranz @ 2021-01-15 12:52 UTC (permalink / raw)
To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev
The kernel test robot kindly pointed out that Global 2 support in
mv88e6xxx is optional.
This also made me realize that we should verify that the hardware
actually supports LAG offloading before trying to configure it.
v1 -> v2:
- Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
- Simplify _has_lag predicate (Vladimir).
Tobias Waldekranz (2):
net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters
net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware
drivers/net/dsa/mv88e6xxx/chip.c | 6 +++++-
drivers/net/dsa/mv88e6xxx/chip.h | 5 +++++
drivers/net/dsa/mv88e6xxx/global2.h | 12 ++++++++++++
3 files changed, 22 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters
2021-01-15 12:52 [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz
@ 2021-01-15 12:52 ` Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz
2021-01-15 23:02 ` [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Jakub Kicinski
2 siblings, 0 replies; 10+ messages in thread
From: Tobias Waldekranz @ 2021-01-15 12:52 UTC (permalink / raw)
To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev
Support for Global 2 registers is build-time optional. In the case
where it was not enabled the build would fail as no "dummy"
implementation of these functions was available.
Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support")
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Tested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/net/dsa/mv88e6xxx/global2.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 60febaf4da76..253a79582a1d 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -525,6 +525,18 @@ static inline int mv88e6xxx_g2_trunk_clear(struct mv88e6xxx_chip *chip)
return -EOPNOTSUPP;
}
+static inline int mv88e6xxx_g2_trunk_mask_write(struct mv88e6xxx_chip *chip,
+ int num, bool hash, u16 mask)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int mv88e6xxx_g2_trunk_mapping_write(struct mv88e6xxx_chip *chip,
+ int id, u16 map)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int mv88e6xxx_g2_device_mapping_write(struct mv88e6xxx_chip *chip,
int target, int port)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware
2021-01-15 12:52 [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz
@ 2021-01-15 12:52 ` Tobias Waldekranz
2021-01-15 14:14 ` Vladimir Oltean
2021-01-15 23:02 ` [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Jakub Kicinski
2 siblings, 1 reply; 10+ messages in thread
From: Tobias Waldekranz @ 2021-01-15 12:52 UTC (permalink / raw)
To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev
There are chips that do have Global 2 registers, and therefore trunk
mapping/mask tables are not available. Refuse the offload as early as
possible on those devices.
Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support")
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 6 +++++-
drivers/net/dsa/mv88e6xxx/chip.h | 5 +++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index dcb1726b68cc..91286d7b12c7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5385,9 +5385,13 @@ static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds,
struct net_device *lag,
struct netdev_lag_upper_info *info)
{
+ struct mv88e6xxx_chip *chip = ds->priv;
struct dsa_port *dp;
int id, members = 0;
+ if (!mv88e6xxx_has_lag(chip))
+ return false;
+
id = dsa_lag_id(ds->dst, lag);
if (id < 0 || id >= ds->num_lag_ids)
return false;
@@ -5727,7 +5731,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
* 5-bit port mode, which we do not support. 640k^W16 ought to
* be enough for anyone.
*/
- ds->num_lag_ids = 16;
+ ds->num_lag_ids = mv88e6xxx_has_lag(chip) ? 16 : 0;
dev_set_drvdata(dev, ds);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 3543055bcb51..788b3f585ef3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -662,6 +662,11 @@ static inline bool mv88e6xxx_has_pvt(struct mv88e6xxx_chip *chip)
return chip->info->pvt;
}
+static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip)
+{
+ return !!chip->info->global2_addr;
+}
+
static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip)
{
return chip->info->num_databases;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware
2021-01-15 12:52 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz
@ 2021-01-15 14:14 ` Vladimir Oltean
0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-15 14:14 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: davem, kuba, andrew, vivien.didelot, f.fainelli, netdev
On Fri, Jan 15, 2021 at 01:52:59PM +0100, Tobias Waldekranz wrote:
> There are chips that do have Global 2 registers, and therefore trunk
> mapping/mask tables are not available. Refuse the offload as early as
> possible on those devices.
>
> Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support")
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 12:52 [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz
@ 2021-01-15 23:02 ` Jakub Kicinski
2021-01-15 23:24 ` Vladimir Oltean
2 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-01-15 23:02 UTC (permalink / raw)
To: Tobias Waldekranz
Cc: davem, andrew, vivien.didelot, f.fainelli, olteanv, netdev
On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
> The kernel test robot kindly pointed out that Global 2 support in
> mv88e6xxx is optional.
>
> This also made me realize that we should verify that the hardware
> actually supports LAG offloading before trying to configure it.
>
> v1 -> v2:
> - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
> - Simplify _has_lag predicate (Vladimir).
If I'm reading the discussion on v1 right there will be a v3,
LMK if I got it wrong.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 23:02 ` [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Jakub Kicinski
@ 2021-01-15 23:24 ` Vladimir Oltean
2021-01-15 23:46 ` Jakub Kicinski
0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-15 23:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tobias Waldekranz, davem, andrew, vivien.didelot, f.fainelli, netdev
On Fri, Jan 15, 2021 at 03:02:46PM -0800, Jakub Kicinski wrote:
> On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
> > The kernel test robot kindly pointed out that Global 2 support in
> > mv88e6xxx is optional.
> >
> > This also made me realize that we should verify that the hardware
> > actually supports LAG offloading before trying to configure it.
> >
> > v1 -> v2:
> > - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
> > - Simplify _has_lag predicate (Vladimir).
>
> If I'm reading the discussion on v1 right there will be a v3,
> LMK if I got it wrong.
I don't think a v3 was supposed to be coming, what made you think that?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 23:24 ` Vladimir Oltean
@ 2021-01-15 23:46 ` Jakub Kicinski
2021-01-15 23:55 ` Vladimir Oltean
2021-01-16 0:12 ` Tobias Waldekranz
0 siblings, 2 replies; 10+ messages in thread
From: Jakub Kicinski @ 2021-01-15 23:46 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Tobias Waldekranz, davem, andrew, vivien.didelot, f.fainelli, netdev
On Sat, 16 Jan 2021 01:24:35 +0200 Vladimir Oltean wrote:
> On Fri, Jan 15, 2021 at 03:02:46PM -0800, Jakub Kicinski wrote:
> > On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
> > > The kernel test robot kindly pointed out that Global 2 support in
> > > mv88e6xxx is optional.
> > >
> > > This also made me realize that we should verify that the hardware
> > > actually supports LAG offloading before trying to configure it.
> > >
> > > v1 -> v2:
> > > - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
> > > - Simplify _has_lag predicate (Vladimir).
> >
> > If I'm reading the discussion on v1 right there will be a v3,
> > LMK if I got it wrong.
>
> I don't think a v3 was supposed to be coming, what made you think that?
I thought you concluded that the entire CONFIG_NET_DSA_MV88E6XXX_GLOBAL2
should go, you said:
> So, roughly, you save 10%/13k. That hardly justifies the complexity IMO.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 23:46 ` Jakub Kicinski
@ 2021-01-15 23:55 ` Vladimir Oltean
2021-01-16 0:13 ` Jakub Kicinski
2021-01-16 0:12 ` Tobias Waldekranz
1 sibling, 1 reply; 10+ messages in thread
From: Vladimir Oltean @ 2021-01-15 23:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tobias Waldekranz, davem, andrew, vivien.didelot, f.fainelli, netdev
On Fri, Jan 15, 2021 at 03:46:22PM -0800, Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 01:24:35 +0200 Vladimir Oltean wrote:
> > On Fri, Jan 15, 2021 at 03:02:46PM -0800, Jakub Kicinski wrote:
> > > On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
> > > > The kernel test robot kindly pointed out that Global 2 support in
> > > > mv88e6xxx is optional.
> > > >
> > > > This also made me realize that we should verify that the hardware
> > > > actually supports LAG offloading before trying to configure it.
> > > >
> > > > v1 -> v2:
> > > > - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
> > > > - Simplify _has_lag predicate (Vladimir).
> > >
> > > If I'm reading the discussion on v1 right there will be a v3,
> > > LMK if I got it wrong.
> >
> > I don't think a v3 was supposed to be coming, what made you think that?
>
> I thought you concluded that the entire CONFIG_NET_DSA_MV88E6XXX_GLOBAL2
> should go, you said:
>
> > So, roughly, you save 10%/13k. That hardly justifies the complexity IMO.
That would be the first time that I hear of fixing a build failure due
to a missing shim by refactoring a driver... Punctual issue, punctual
fix, no?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 23:46 ` Jakub Kicinski
2021-01-15 23:55 ` Vladimir Oltean
@ 2021-01-16 0:12 ` Tobias Waldekranz
1 sibling, 0 replies; 10+ messages in thread
From: Tobias Waldekranz @ 2021-01-16 0:12 UTC (permalink / raw)
To: Jakub Kicinski, Vladimir Oltean
Cc: davem, andrew, vivien.didelot, f.fainelli, netdev
On Fri, Jan 15, 2021 at 15:46, Jakub Kicinski <kuba@kernel.org> wrote:
> On Sat, 16 Jan 2021 01:24:35 +0200 Vladimir Oltean wrote:
>> On Fri, Jan 15, 2021 at 03:02:46PM -0800, Jakub Kicinski wrote:
>> > On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
>> > > The kernel test robot kindly pointed out that Global 2 support in
>> > > mv88e6xxx is optional.
>> > >
>> > > This also made me realize that we should verify that the hardware
>> > > actually supports LAG offloading before trying to configure it.
>> > >
>> > > v1 -> v2:
>> > > - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
>> > > - Simplify _has_lag predicate (Vladimir).
>> >
>> > If I'm reading the discussion on v1 right there will be a v3,
>> > LMK if I got it wrong.
>>
>> I don't think a v3 was supposed to be coming, what made you think that?
>
> I thought you concluded that the entire CONFIG_NET_DSA_MV88E6XXX_GLOBAL2
> should go, you said:
>
>> So, roughly, you save 10%/13k. That hardly justifies the complexity IMO.
Well, based on what Andrew said, my guess is that that is where we will
end up.
But since at the moment net-next does not build for configs without
Global2-support, I would really appreciate it if this series could be
applied to solve that immediate problem.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes
2021-01-15 23:55 ` Vladimir Oltean
@ 2021-01-16 0:13 ` Jakub Kicinski
0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2021-01-16 0:13 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Tobias Waldekranz, davem, andrew, vivien.didelot, f.fainelli, netdev
On Sat, 16 Jan 2021 01:55:47 +0200 Vladimir Oltean wrote:
> On Fri, Jan 15, 2021 at 03:46:22PM -0800, Jakub Kicinski wrote:
> > On Sat, 16 Jan 2021 01:24:35 +0200 Vladimir Oltean wrote:
> > > On Fri, Jan 15, 2021 at 03:02:46PM -0800, Jakub Kicinski wrote:
> > > > On Fri, 15 Jan 2021 13:52:57 +0100 Tobias Waldekranz wrote:
> > > > > The kernel test robot kindly pointed out that Global 2 support in
> > > > > mv88e6xxx is optional.
> > > > >
> > > > > This also made me realize that we should verify that the hardware
> > > > > actually supports LAG offloading before trying to configure it.
> > > > >
> > > > > v1 -> v2:
> > > > > - Do not allocate LAG ID mappings on unsupported hardware (Vladimir).
> > > > > - Simplify _has_lag predicate (Vladimir).
> > > >
> > > > If I'm reading the discussion on v1 right there will be a v3,
> > > > LMK if I got it wrong.
> > >
> > > I don't think a v3 was supposed to be coming, what made you think that?
> >
> > I thought you concluded that the entire CONFIG_NET_DSA_MV88E6XXX_GLOBAL2
> > should go, you said:
> >
> > > So, roughly, you save 10%/13k. That hardly justifies the complexity IMO.
>
> That would be the first time that I hear of fixing a build failure due
> to a missing shim by refactoring a driver... Punctual issue, punctual
> fix, no?
Sure, without knowing the driver it's hard to tell if it's a matter of
removing those stubs in the header, or more work, hence the question.
Applied now, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-16 0:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15 12:52 [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz
2021-01-15 12:52 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz
2021-01-15 14:14 ` Vladimir Oltean
2021-01-15 23:02 ` [PATCH v2 net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Jakub Kicinski
2021-01-15 23:24 ` Vladimir Oltean
2021-01-15 23:46 ` Jakub Kicinski
2021-01-15 23:55 ` Vladimir Oltean
2021-01-16 0:13 ` Jakub Kicinski
2021-01-16 0:12 ` Tobias Waldekranz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.