From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Simon Horman <horms@kernel.org>, Jakub Kicinski <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>,
Andy Gospodarek <andy@greyhouse.net>,
netdev@vger.kernel.org
Subject: Re: [PATCH] bonding: Always assign be16 value to vlan_proto
Date: Fri, 21 Apr 2023 15:34:18 -0700 [thread overview]
Message-ID: <29611.1682116458@famine> (raw)
In-Reply-To: <20230421091737.deetnyj6cakrn3mg@skbuf>
Vladimir Oltean <olteanv@gmail.com> wrote:
>On Fri, Apr 21, 2023 at 09:01:34AM +0200, Simon Horman wrote:
>> Hi Jay and Vladimir,
>>
>> Thanks for your review.
>>
>> Firstly, sorry for the distraction about the VLAN_N_VID math. I agree it
>> was incorrect. I had an out by one bug in my thought process which was
>> about 0x0fff instead of 0x1000.
>>
>> Secondly, sorry for missing the central issue that it is a bit weird
>> to use a VID related value as a sentinel for a protocol field.
>> I agree it would be best to chose a different value.
>>
>> In reference to the list of EtherTypes [1]. I think 0 might be ok,
>> but perhaps not ideal as technically it means a value of 0 for the
>> IEEE802.3 Length Field (although perhaps it can never mean that in this
>> context).
>>
>> OTOH, 0xffff, is 'reserved' ([1] references RFC1701 [2]),
>> so perhaps it is a good choice.
>>
>> In any case, I'm open to suggestions.
>> I'll probably hold off until the v6.5 cycle before reposting,
>> unless -rc8 appears next week. I'd rather not rush this one
>> given that I seem to have already got it wrong once.
>>
>> [1] https://www.iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml#ieee-802-numbers-1
>> [2] https://www.rfc-editor.org/rfc/rfc1701.html
>
>Any value would work as long as it's not a valid VLAN protocol.
>I would #define BOND_VLAN_PROTO_NONE htons(0xffff) and use that.
All of the above is fine with me; this isn't an urgent change.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
prev parent reply other threads:[~2023-04-21 22:44 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-20 15:29 [PATCH] bonding: Always assign be16 value to vlan_proto Simon Horman
2023-04-20 19:47 ` Jay Vosburgh
2023-04-20 20:23 ` Vladimir Oltean
2023-04-20 21:23 ` Jay Vosburgh
2023-04-21 7:01 ` Simon Horman
2023-04-21 9:17 ` Vladimir Oltean
2023-04-21 22:34 ` Jay Vosburgh [this message]
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=29611.1682116458@famine \
--to=jay.vosburgh@canonical.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.