Re: [PATCH V2 4/7] net/mlx4_en: Set max rate-limit for a TC

Amir Vadai <amirv@xxxxxxxxxxxx> · Tue, 27 Mar 2012 14:03:57 +0200

On 03/26/2012 08:55 PM, John Fastabend wrote:

On 3/26/2012 10:00 AM, Or Gerlitz wrote:

On Mon, Mar 26, 2012 at 4:46 PM, Ben Hutchings
<bhutchings@xxxxxxxxxxxxxx>  wrote:

We used sysfs since max bw isn't part of the ETS / DCBX NL support, and we're
open to other suggestions to add generic support for max bw, e.g add call to
the DCBX NL API.

netlink interfaces are generally easily extensible and it doesn't make
sense to me to augment such an interface through sysfs.  Perhaps you're
concerned that netlink extensions won't be supported in older kernel
versions running your OOT driver?  That's unfortunate, but let's not
standardise an ugly interface based on a temporary problem like that.

As written above, that was done since ratelimit isn't part of ETS, we can
that through netlink extensions that you mentioned, if this is the preffered
way to go, David? Eric? Ben - could you provide pointer to these extensions?

Or.
--

I think I original suggested it didn't belong in DCBNL because it
wasn't part of ETS (802.1Qaz). But it _is_ a traffic selection
algorithm and could fall into the vendor specific part of 802.1Q.

I would suggest either adding it as an option to mqprio to take
a max bandwidth. The advantage here is it would be tied in with
the usual QOS tooling 'tc'.

# tc qdisc add dev eth3 root mqprio help
Usage: ... mqprio [num_tc NUMBER] [map P0 P1 ...]
                   [queues count1@offset1 count2@offset2 ...] [hw 1|0]
		  [max_rate rate@tc ...]

This could be elegant, but since tc here is a logical traffic class and
ratelimit in our context is an attribute of ETS TC, it could be
problematic.

Or extending DCBNL being careful not to break backwards
compatibility. I tend to think extending mqprio is cleaner but
a DCBNL extension could likely work as well. Would need a
'DCBNL_IEEE_SET_MAXRATE' and 'DCBNL_IEEE_GET_MAXRATE' for this
I expect.

I do prefer this option. I'm checking now if it is possible to add
infrastructure for a vendor specific netlink command. It could be
connected by lldpad to the vendor TLV in DCBX.

.John

I will send a V3 of this patchset without the ratelimit patch, to make 

sure the patchset will get into net-next on time.

Thanks,
Amir
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html