netdev - Re: [PATCH net-next 1/3] net/dcb: Add IEEE QCN attribute

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]

Date:	Thu, 5 Mar 2015 08:53:43 +0200
From:	Or Gerlitz <gerlitz.or@...il.com>
To:	John Fastabend <john.fastabend@...il.com>
Cc:	Or Gerlitz <ogerlitz@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>,
	John Fastabend <john.r.fastabend@...el.com>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Amir Vadai <amirv@...lanox.com>, Tal Alon <talal@...lanox.com>,
	Shani Michaeli <shanim@...lanox.com>,
	Shachar Raindel <raindel@...lanox.com>
Subject: Re: [PATCH net-next 1/3] net/dcb: Add IEEE QCN attribute

On Wed, Mar 4, 2015 at 7:19 PM, John Fastabend <john.fastabend@...il.com> wrote:
> On 03/04/2015 04:51 AM, Or Gerlitz wrote:
>>
>> From: Shani Michaeli <shanim@...lanox.com>
>>
>> As specified in 802.1Qau spec. Add this optional attribute to the
>> DCB netlink layer. To allow for application to use the new attribute,
>> NIC drivers should implement and register the  callbacks ieee_getqcn,
>> ieee_setqcn and ieee_getqcnstats.
>>
>> The QCN attribute holds a set of parameters for management, and
>> a set of statistics to provide informative data on Congestion-Control
>> defined by this spec.
>>
>> Signed-off-by: Shani Michaeli <shanim@...lanox.com>
>> Signed-off-by: Shachar Raindel <raindel@...lanox.com>
>> Signed-off-by: Or Gerlitz <ogerlitz@...lanox.com>

> Looks good to me. Do you have a QCN enabled switch? I looked at
> implementing this awhile ago but didn't have any switch support so
> I never did it.

I'l let Shachar to address the testing and the MIB questions.

> Also do you have a user space client to configure this? I would like
> it if someone wanted to add support to lldpad/dcbtool.

Sure, we have some netlink (python scripts) code to configure/read
this towards the kernel.


> [...]
>> +
>> +/* This structure contains the IEEE 802.1Qau QCN managed object.
>> + *
>> + *@..._enable: enable QCN RP
>> + *@...p_max_rps: maximum number of RPs allowed for this CNPV on this port
>> + *@..._time_reset: time between rate increases if no CNMs received.
>> + *                given in u-seconds
>> + *@..._byte_reset: transmitted data between rate increases if no CNMs
>> received.
>> + *                given in Bytes
>> + *@..._threshold: The number of times rpByteStage or rpTimeStage can
>> count
>> + *                before RP rate control state machine advances states
>> + *@..._max_rate: the maxinun rate, in Mbits per second,
>> + *              at which an RP can transmit
>> + *@..._ai_rate: The rate, in Mbits per second,
>> + *             used to increase rpTargetRate in the RPR_ACTIVE_INCREASE
>> + *@..._hai_rate: The rate, in Mbits per second,
>> + *              used to increase rpTargetRate in the RPR_HYPER_INCREASE
>> state
>> + *@..._gd: Upon CNM receive, flow rate is limited to (Fb/Gd)*CurrentRate.
>> + *        rpgGd is given as log2(Gd), where Gd may only be powers of 2
>> + *@..._min_dec_fac: The minimum factor by which the current transmit rate
>> + *                 can be changed by reception of a CNM.
>> + *                 value is given as percentage (1-100)
>> + *@..._min_rate: The minimum value, in bits per second, for rate to limit
>> + *@...d_state_machine: The state of the congestion notification domain
>> + *                    defense state machine, as defined by IEEE 802.3Qau
>> + *                    section 32.1.1. In the interior ready state,
>> + *                    the QCN capable hardware may add CN-TAG TLV to the
>> + *                    outgoing traffic, to specifically identify outgoing
>> + *                    flows.
>> + */
>
>
> I'm assuming this structure maps to an IEEE MIB?

yep, I guess so

> Its a rather large structure for a single netlink type but this seems
> to be how we built
> the dcbnl interface and if it does seem logical that the structure is
> one logical block, meaning you need to supply all fields.

> [...]
>> +       if (ops->ieee_getqcn) {
>> +               struct ieee_qcn qcn;

> you might consider adding a newline here it  is the best practice for
> new code although dcbnl has plenty of examples where it doesn't use
> this convention.

OK, will add this here and in the other places where used that practice.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html