linux-kernel - Re: [RFC Patch net-next v2 05/15] net: dsa: microchip: move the port mirror to ksz

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: <5122bb72cee5e0a3fa3cb66d55f141b66f145e7b.camel@microchip.com>
Date:   Wed, 15 Jun 2022 06:40:39 +0000
From:   <Arun.Ramadoss@...rochip.com>
To:     <olteanv@...il.com>
CC:     <andrew@...n.ch>, <linux-kernel@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>, <vivien.didelot@...il.com>,
        <linux@...linux.org.uk>, <f.fainelli@...il.com>, <kuba@...nel.org>,
        <edumazet@...gle.com>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
        <davem@...emloft.net>
Subject: Re: [RFC Patch net-next v2 05/15] net: dsa: microchip: move the port
 mirror to ksz_common

On Mon, 2022-06-13 at 12:28 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, May 30, 2022 at 04:12:47PM +0530, Arun Ramadoss wrote:
> > This patch updates the common port mirror add/del dsa_switch_ops in
> > ksz_common.c. The individual switches implementation is executed
> > based
> > on the ksz_dev_ops function pointers.
> > 
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > ---
> 
> Reviewed-by: Vladimir Oltean <olteanv@...il.com>
> 
> >  drivers/net/dsa/microchip/ksz8795.c    | 13 ++++++-------
> >  drivers/net/dsa/microchip/ksz9477.c    | 12 ++++++------
> >  drivers/net/dsa/microchip/ksz_common.c | 25
> > +++++++++++++++++++++++++
> >  drivers/net/dsa/microchip/ksz_common.h | 10 ++++++++++
> >  4 files changed, 47 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 157d69e46793..8657b520b336 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1089,12 +1089,10 @@ static int ksz8_port_vlan_del(struct
> > ksz_device *dev, int port,
> >       return 0;
> >  }
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index a1fef9e4e36c..1ed4cc94795e 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -994,6 +994,31 @@ int ksz_port_vlan_del(struct dsa_switch *ds,
> > int port,
> >  }
> >  EXPORT_SYMBOL_GPL(ksz_port_vlan_del);
> > 
> > +int ksz_port_mirror_add(struct dsa_switch *ds, int port,
> > +                     struct dsa_mall_mirror_tc_entry *mirror,
> > +                     bool ingress, struct netlink_ext_ack *extack)
> > +{
> > +     struct ksz_device *dev = ds->priv;
> > +     int ret = -EOPNOTSUPP;
> > +
> > +     if (dev->dev_ops->mirror_add)
> > +             ret = dev->dev_ops->mirror_add(dev, port, mirror,
> > ingress,
> > +                                            extack);
> > +
> > +     return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(ksz_port_mirror_add);
> 
> Just as a minor style comment, take it or leave it.
> 
> If you switch the function pointer presence check, you reduce the
> indentation of the long statement, making it fit a single line, and
> you
> eliminate the need for a "ret" variable:
> 
>         if (!dev->dev_ops->mirror_add)
>                 return -EOPNOTSUPP;
> 
>         return dev->dev_ops->mirror_add(dev, port, mirror, ingress,
> extack);

Yes it reduces the indentation. I will update it for other checks too.

> > +
> > +void ksz_port_mirror_del(struct dsa_switch *ds, int port,
> > +                      struct dsa_mall_mirror_tc_entry *mirror)
> > +{
> > +     struct ksz_device *dev = ds->priv;
> > +
> > +     if (dev->dev_ops->mirror_del)
> > +             dev->dev_ops->mirror_del(dev, port, mirror);
> > +}
> > +EXPORT_SYMBOL_GPL(ksz_port_mirror_del);
> > +
> >  static int ksz_switch_detect(struct ksz_device *dev)
> >  {
> >       u8 id1, id2;
> > 
> > 
> > --
> > 2.36.1
> > 
> 
>