netdev - Re: [RFC net-next 3/5] net: dsa: mv88e6xxx: MQPRIO support

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: <d2d289ca-b40c-4e83-8fb6-ccf53d572642@lunn.ch>
Date: Sat, 27 Sep 2025 19:36:06 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Luke Howard <lukeh@...l.com>
Cc: netdev@...r.kernel.org, vladimir.oltean@....com, kieran@...nda.com,
	jcschroeder@...il.com, Max Hunter <max@...tershome.org>
Subject: Re: [RFC net-next 3/5] net: dsa: mv88e6xxx: MQPRIO support

> > 
> >> + *
> >> + * @param chip Marvell switch chip instance
> >> + * @param hilimit Maximum frame size allowed for AVB Class A frames
> >> + *
> >> + * @return 0 on success, or a negative error value otherwise
> >> + */
> > 
> > kerneldoc wants a : after return.
> 
> Should also be for @param then? Seems fairly inconsistent on a brief survey of other drivers.
 
Ah, sorry. I missed you did not start the documentation correctly, so
it is not even considered kerneldoc.

You need

/**

to start the block.

https://docs.kernel.org/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments

> >> +static int mv88e6xxx_avb_set_hilimit(struct mv88e6xxx_chip *chip, u16 hilimit)
> >> +{
> >> + u16 data;
> >> + int err;
> >> +
> >> + if (hilimit > MV88E6XXX_AVB_CFG_HI_LIMIT_MASK)
> >> + return -EINVAL;
> > 
> > Does it make sense to check it against the MTU? Does it matter if it
> > is bigger than the MTU?
> 
> I don’t think so; this is hicredit in tc-cbs(8).

O.K, i've no real knowledge of AVB...

> >> + * - because the Netlink API has no way to distinguish between FDB/MDB
> >> + *  entries managed by SRP from those that are not, the
> >> + *  "marvell,mv88e6xxx-avb-mode" device tree property controls whether
> >> + *  a FDB or MDB entry is required in order for AVB frames to egress.
> > 
> > We probably need to think about this. What about other devices which
> > require this? Would it be better to extend the netlink API to pass
> > some sort of owner? If i remember correctly, routes passed by netlink
> > can indicate which daemon is responsible for it, quagga, zebra, bgp
> > etc.
> 

> An additional flag to Netlink when adding a FDB or MDB entry would
> certainly be cleaner. I’m not sure what other devices do, I suspect
> some support similar functionality but do not have driver support,
> because most SRP implementations have managed the switch
> directly. (Mine [1] uses standard kernel interfaces.)

Having an open user space side helps get such an extra attribute
added.

Would the kernel bridge have any use of this? It is normal to add a
feature to the kernel, and then offload it to hardware. 

	Andrew