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

[Andrew Lunn <andrew@lunn.ch>]

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@gmail.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