netdev - Re: [PATCH] net: dsa: mv88e6060: report max mtu 1536

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABikg9zb7z8p7tE0H+fpmB_NSK3YVS-Sy4sqWbihziFdPBoL+Q@mail.gmail.com>
Date:   Wed, 10 Aug 2022 15:00:20 +0300
From:   Sergei Antonov <saproj@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH] net: dsa: mv88e6060: report max mtu 1536

On Wed, 10 Aug 2022 at 13:08, Vladimir Oltean <olteanv@...il.com> wrote:
>
> Hi Sergei,
>
> On Wed, Aug 10, 2022 at 11:27:45AM +0300, Sergei Antonov wrote:
> > This driver sets the MaxFrameSize bit to 1 during setup,
> > see GLOBAL_CONTROL_MAX_FRAME_1536 in mv88e6060_setup_global().
> > Thus MTU is always 1536.
> > Introduce mv88e6060_port_max_mtu() to report it back to system.
> >
> > Signed-off-by: Sergei Antonov <saproj@...il.com>
> > CC: Vladimir Oltean <olteanv@...il.com>
> > CC: Florian Fainelli <f.fainelli@...il.com>
> > ---
> >  drivers/net/dsa/mv88e6060.c | 7 ++++++-
> >  drivers/net/dsa/mv88e6060.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
> > index a4c6eb9a52d0..c53734379b96 100644
> > --- a/drivers/net/dsa/mv88e6060.c
> > +++ b/drivers/net/dsa/mv88e6060.c
> > @@ -160,7 +160,6 @@ static int mv88e6060_setup_addr(struct mv88e6060_priv *priv)
> >       u16 val;
> >
> >       eth_random_addr(addr);
> > -
>
> Extraneous change.

Thanks! My fault.

> >       val = addr[0] << 8 | addr[1];
> >
> >       /* The multicast bit is always transmitted as a zero, so the switch uses
> > @@ -212,6 +211,11 @@ static int mv88e6060_setup(struct dsa_switch *ds)
> >       return 0;
> >  }
> >
> > +static int mv88e6060_port_max_mtu(struct dsa_switch *ds, int port)
> > +{
> > +     return MV88E6060_MAX_MTU;
> > +}
>
> Does this solve any problem? It's ok for the hardware MTU to be higher
> than advertised. The problem is when the hardware doesn't accept what
> the stack thinks it should.

I need some time to reconstruct the problem. IIRC there was an attempt
to set MTU 1504 (1500 + a switch overhead), but can not reproduce it
at the moment.

> > +
> >  static int mv88e6060_port_to_phy_addr(int port)
> >  {
> >       if (port >= 0 && port < MV88E6060_PORTS)
> > @@ -247,6 +251,7 @@ mv88e6060_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
> >  static const struct dsa_switch_ops mv88e6060_switch_ops = {
> >       .get_tag_protocol = mv88e6060_get_tag_protocol,
> >       .setup          = mv88e6060_setup,
> > +     .port_max_mtu   = mv88e6060_port_max_mtu,
> >       .phy_read       = mv88e6060_phy_read,
> >       .phy_write      = mv88e6060_phy_write,
> >  };
> > diff --git a/drivers/net/dsa/mv88e6060.h b/drivers/net/dsa/mv88e6060.h
> > index 6c13c2421b64..382fe462fb2d 100644
> > --- a/drivers/net/dsa/mv88e6060.h
> > +++ b/drivers/net/dsa/mv88e6060.h
> > @@ -11,6 +11,7 @@
> >  #define __MV88E6060_H
> >
> >  #define MV88E6060_PORTS      6
> > +#define MV88E6060_MAX_MTU    1536
> >
> >  #define REG_PORT(p)          (0x8 + (p))
> >  #define PORT_STATUS          0x00
> > --
> > 2.32.0
> >
>
> You're the first person to submit a patch on mv88e6060 that I see.
> Is there a board with this switch available somewhere? Does the driver
> still work?

Very nice to get your feedback. Because, yes, I am working with a
device which has mv88e6060, it is called MOXA NPort 6610.

The driver works now. There was one problem which I had to workaround.
Inside my device only ports 2 and 5 are used, so I initially wrote in
.dts:
        switch@0 {
                compatible = "marvell,mv88e6060";
                reg = <16>;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@2 {
                                reg = <2>;
                                label = "lan2";
                        };

                        port@5 {
                                reg = <5>;
                                label = "cpu";
                                ethernet = <&mac1>;
                        };
                };
        };
and the driver crashed in mv88e6060_setup_port() on a null pointer.
Two workarounds are possible:
1. Describe ports 0, 1, 3, 4 in .dts too.
2. Insert this code at the beginning of mv88e6060_setup_port():
if(!dsa_is_cpu_port(priv->ds, p) && !dsa_to_port(priv->ds, p)->cpu_dp)
    return 0;
'cpu_dp' was the null pointer the driver crashed at.

One more observation. Generating and setting a random MAC in
mv88e6060_setup_addr() is not necessary - the switch works without it
(at least in my case).