From: Jianhua Xie <jianhua.xie@freescale.com>
To: Veaceslav Falico <vfalico@gmail.com>
Cc: Jay Vosburgh <jay.vosburgh@canonical.com>,
David Miller <davem@davemloft.net>, <netdev@vger.kernel.org>,
<andy@greyhouse.net>, Jianhua Xie <jianhua.xie@freescale.com>
Subject: Re: [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key
Date: Sun, 16 Nov 2014 16:45:45 +0800 [thread overview]
Message-ID: <54686439.7070901@freescale.com> (raw)
In-Reply-To: <20141112112002.GA27653@raspberrypi>
在 2014年11月12日 19:20, Veaceslav Falico 写道:
> On Wed, Nov 12, 2014 at 05:53:41PM +0800, Jianhua Xie wrote:
>> Thanks you two for the valuable comments.
>>
>> If my understanding is right, it is encouraged to use a counter
>> rather than a bitmask for the speed field, right?
>>
>> if yes, how many bits are better to use for current speed and
>> future speed (like 100Gbps/400Gbps and etc.)? I am not sure
>> that 5 bits are enough (2**5=32) or not. And I am clear to keep
>> "the duplex bit in the key " in my mind.
>>
>> if not, what's your recommendation please?
>
> As it's visible to bonding only, I guess a simple enum should do the
> trick.
> No need to invent something special, and it'll fit nicely with other
> enums
> from AD.
Thanks comments from Jay Vosburgh and Veaceslav Falico. However, my
method can also work, and also compatible with current bonding driver.
But Veaceslav Falico's method is better than mine. I am glad to take his
advice. I will use an enum to instead of AD_LINK_SPEED_BITMASK micros
which are based on bitmask.
I also thank Miller for the kindly reminder on "please don't top-post".
Thank & Best Regards,
Jianhua
>
>>
>> Thanks & Best Regards,
>> Jianhua
>>
>> 在 2014年11月12日 03:47, Jay Vosburgh 写道:
>>> David Miller <davem@davemloft.net> wrote:
>>>
>>>> From: Xie Jianhua <Jianhua.Xie@freescale.com>
>>>> Date: Mon, 10 Nov 2014 15:16:40 +0800
>>>>
>>>>> From: Jianhua Xie <Jianhua.Xie@freescale.com>
>>>>>
>>>>> Port Key was determined as 16 bits according to the link speed,
>>>>> duplex and user key (which is yet not supported), in which key
>>>>> speed was 5 bits for 1Mbps/10Mbps/100Mbps/1Gbps/10Gbps as below:
>>>>> --------------------------------------------------------------
>>>>> Port key :| User key | Speed | Duplex|
>>>>> --------------------------------------------------------------
>>>>> 16 6 1 0
>>>>> This patch is expanding speed type from 5 bits to 9 bits for other
>>>>> speed 2.5Gbps/20Gbps/40Gbps/56Gbps and shrinking user key from 10
>>>>> bits to 6 bits. New Port Key looks like below:
>>>>> --------------------------------------------------------------
>>>>> Port key :| User key | Speed | Duplex|
>>>>> --------------------------------------------------------------
>>>>> 16 10 1 0
>>>>>
>>>> Do we determine the layout of this value all ourselves?
>>> Yes, we do. The precise format of the port key is not defined
>>> by the standard; IEEE 802.1AX 5.3.5, "Capability identification":
>>>
>>> "A given Key value is meaningful only in the context of the System that
>>> allocates it; there is no global significance to Key values."
>>>
>>> and
>>>
>>> "When a System assigns an operational Key value to a set of ports, it
>>> signifies that, in the absence of other constraints, the current
>>> operational state of the set of ports allows any subset of that set of
>>> ports (including the entire set) to be aggregated together from the
>>> perspective of the System making the assignment."
>>>
>>> So, basically, it's a magic cookie that indicates that all ports
>>> on a particular system with the same key value are suitable to be
>>> aggregated together.
>>>
>>>> If not, then is it exported to anything user-visible that we
>>>> might be breaking?
>>> The key values are not user-visible, and the "user" settable
>>> portion of the key has never been implemented.
>>>
>>>> If it is private, it makes no sense to use a bitmask for the speed.
>>>> We should instead change the field to be some numerically increasing
>>>> value.
>>>>
>>>> Otherwise we'll run out of bits again and keep having to adjust the
>>>> field layout more often than we really need to.
>>> Agreed.
>>>
>>> Also note that there are some internal dependencies within
>>> bonding on the format; in particular the duplex bit in the key is used
>>> to determine if a port is LACP-capable, and that functionality needs to
>>> be preserved.
>>>
>>> -J
>>>
>>> ---
>>> -Jay Vosburgh, jay.vosburgh@canonical.com
>>
next prev parent reply other threads:[~2014-11-16 8:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-10 7:16 [PATCH v1 net-next 0/2] bonding: Introduce 4 AD link speed Xie Jianhua
2014-11-10 7:16 ` [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key Xie Jianhua
2014-11-11 18:53 ` David Miller
2014-11-11 19:47 ` Jay Vosburgh
2014-11-12 9:53 ` Jianhua Xie
2014-11-12 11:20 ` Veaceslav Falico
2014-11-16 8:45 ` Jianhua Xie [this message]
2014-11-12 17:43 ` David Miller
2014-11-10 7:16 ` [PATCH v1 net-next 2/2] bonding: Introduce 4 AD link speed to fix agg_bandwidth Xie Jianhua
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54686439.7070901@freescale.com \
--to=jianhua.xie@freescale.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=jay.vosburgh@canonical.com \
--cc=netdev@vger.kernel.org \
--cc=vfalico@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).