netdev - Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: Add support for port mirroring

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20191023011802.GI5707@lunn.ch>
Date:   Wed, 23 Oct 2019 03:18:02 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Iwan R Timmer <irtimmer@...il.com>
Cc:     netdev@...r.kernel.org, vivien.didelot@...il.com,
        f.fainelli@...il.com, davem@...emloft.net
Subject: Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: Add support for
 port mirroring

On Mon, Oct 21, 2019 at 11:01:43PM +0200, Iwan R Timmer wrote:
> Add support for configuring port mirroring through the cls_matchall
> classifier. We do a full ingress and/or egress capture towards a
> capture port. It allows setting a different capture port for ingress
> and egress traffic.
> 
> It keeps track of the mirrored ports and the destination ports to
> prevent changes to the capture port while other ports are being
> mirrored.
> 
> Signed-off-by: Iwan R Timmer <irtimmer@...il.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c    | 70 +++++++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/chip.h    |  6 +++
>  drivers/net/dsa/mv88e6xxx/global1.c | 11 ++++-
>  drivers/net/dsa/mv88e6xxx/port.c    | 29 ++++++++++++
>  drivers/net/dsa/mv88e6xxx/port.h    |  2 +
>  5 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index e9735346838d..14e71bc98513 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4921,6 +4921,74 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
>  	return err;
>  }
>  
> +static int mv88e6xxx_port_mirror_add(struct dsa_switch *ds, int port,
> +				     struct dsa_mall_mirror_tc_entry *mirror,
> +				     bool ingress)

This bool is defined by the API to the DSA core, so we cannot do much
to make it easier to understand.

> +	err = mv88e6xxx_port_set_mirror(chip, port, ingress, true);

but here we could use enums for both ingress and this second true.
Maybe the enum i suggested for the previous patch could be more
generic and just represent ingress/egress?

> @@ -228,6 +228,8 @@ struct mv88e6xxx_port {
>  	u64 vtu_miss_violation;
>  	u8 cmode;
>  	unsigned int serdes_irq;
> +	bool mirror_ingress;
> +	bool mirror_egress;
>  };

In terms of structure packing, it might be better to put these two
bools after cmode.

      Andrew