Re: [PATCH net-next v2 1/1] net: dsa: microchip: Add support for bridge port isolation

[Simon Horman <horms@kernel.org>]

On Wed, Feb 21, 2024 at 10:34:29AM +0100, Oleksij Rempel wrote:
> Implement bridge port isolation for KSZ switches. Enabling the isolation
> of switch ports from each other while maintaining connectivity with the
> CPU and other forwarding ports. For instance, to isolate swp1 and swp2
> from each other, use the following commands:
> - bridge link set dev swp1 isolated on
> - bridge link set dev swp2 isolated on
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
> ---
> changes v2:
> - add comments and new lines
> 
>  drivers/net/dsa/microchip/ksz_common.c | 55 +++++++++++++++++++++++---
>  drivers/net/dsa/microchip/ksz_common.h |  1 +
>  2 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 7cd37133ec05..efe54c9492e8 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1898,6 +1898,29 @@ static void ksz_get_strings(struct dsa_switch *ds, int port,
>  	}
>  }
>  
> +/**
> + * ksz_adjust_port_member - Adjust port forwarding rules based on STP state and
> + *			    isolation settings.
> + * @dev: A pointer to the struct ksz_device representing the device.
> + * @port: The port number to adjust.
> +
> + * This function dynamically adjusts the port membership configuration for a
> + * specified port and other device ports, based on Spanning Tree Protocol (STP)
> + * states and port isolation settings. Each port, including the CPU port, has a
> + * membership register, represented as a bitfield, where each bit corresponds
> + * to a port number. A set bit indicates permission to forward frames to that
> + * port. This function iterates over all ports, updating the membership register
> + * to reflect current forwarding permissions:
> + *
> + * 1. Forwards frames only to ports that are part of the same bridge group and
> + *    in the BR_STATE_FORWARDING state.
> + * 2. Takes into account the isolation status of ports; ports in the
> + *    BR_STATE_FORWARDING state with BR_ISOLATED configuration will not forward
> + *    frames to each other, even if they are in the same bridge group.
> + * 3. Ensures that the CPU port is included in the membership based on its
> + *    upstream port configuration, allowing for management and control traffic
> + *    to flow as required.
> + */
>  static void ksz_update_port_member(struct ksz_device *dev, int port)

...

Hi Oleksij,

I haven't looked over this patch closely.
But it does seem to have some Kernel doc problems.

  .../ksz_common.c:1905: warning: bad line:
  .../ksz_common.c:1924: warning: expecting prototype for ksz_adjust_port_member(). Prototype was for ksz_update_port_member() instead

You can observe these by running ./scripts/kernel-doc -none

I'm going to flag this as Changes Requested in patchwork
as typically this kind of problem gets flagged by bots sooner or later.

pw-bot: changes-requested