Re: [PATCH net-next v6 5/9] net: dsa: microchip: add support for different DCB app configurations

[Oleksij Rempel]

Hi Vladimir,

On Thu, Apr 11, 2024 at 02:12:51AM +0300, Vladimir Oltean wrote:
> > +/**
> > + * ksz_dcb_init - Initializes the DCB configuration for a KSZ switch
> > + * @dev: Pointer to the KSZ switch device structure
> > + *
> > + * This function initializes the DCB configuration for a KSZ switch. The global
> > + * DSCP-to-priority mapping table is initialized.
> > + *
> > + * Return: 0 on success, or a negative error code on failure
> > + */
> > +int ksz_dcb_init(struct ksz_device *dev)
> > +{
> > +	int ret;
> > +
> > +	ret = ksz_init_global_dscp_map(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> 
> Sorry for not responding to your previous question about this:
> https://lore.kernel.org/netdev/ZfmJ-O8XMT8oO-TS@xxxxxxxxxxxxxx/
> Simply put, I had a period with not a lot of free time, even for reading
> emails.

No problem. I'm in continues similar state permanently DoSed by my
children, parents, etc... :) 

> I'm on the fence on whether your solution to the "global DSCP-to-prio
> mapping rather than per-port" problem is the right one.
> 
> We try to avoid baking policies into the kernel, no matter how well
> intended the 802.1Q and IETF RFC8325 recommendations are. They are still
> just recommendations and examples, and a particular use case may want to
> configure things completely differently (or as hinted in the Wi-Fi specific
> RFC8325: maybe the administrator doesn't want to assign the higher
> traffic classes, for network control protocols, by using IP DSCP, and
> doesn't want user flows to request DSCP values that would get access to
> these traffic classes. It can indeed be seen as a security concern).
>
> I empathize with the incovenience of having to map the per-netdev dcbnl
> application priority table API with a piece of hardware where that table
> is shared across all ports. But yet, I don't think it is a strong enough
> justification for us to make an exception and say: "yeah, ok, let's not
> even implement .port_set_dscp_prio() to make the thing configurable, but
> let's bake into the kernel a fixed policy that's good for everyone".
>
> No, I think we _need_ the thing to be configurable, and not try so hard
> with the ieee8021q helpers to hardcode things just right in the kernel.

Yes, I agree with you.

ieee8021q helpers are not the attempt to avoid the work needed to
implement global DSCP configuration. The interface is still needed and
we need to agree on how it should be implemented.

The problem which I try to address with ieee8021q helpers are initial
defaults. KSZ8 and KSZ9 families of switches have different initial
defaults. So, if i need to align defaults for this driver, why not to
provide default which are reusable for every one?

> Have you tried the obvious: "every time there is a change to the global
> DSCP mapping table, push the change into the dcbnl app table of all user
> netdevs, so that the user becomes aware of what happens"? Kernel drivers
> can do that, through direct calls to dcb_ieee_setapp(). DSA does it too,
> to probe the initial QoS configuration of the ports and push it to the
> application priority tables.

Hm... what interface should be used for the global DSCP mapping table?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |