From: Nikolay Aleksandrov <nikolay@nvidia.com>
To: Jakub Kicinski <kuba@kernel.org>, David Lamparter <equinox@diac24.net>
Cc: <netdev@vger.kernel.org>, Alexandra Winter <wintera@linux.ibm.com>
Subject: Re: [PATCH] bridge: extend BR_ISOLATE to full split-horizon
Date: Thu, 9 Dec 2021 18:23:25 +0200 [thread overview]
Message-ID: <481128e0-d4d0-3fde-6a9e-65e391bbf64d@nvidia.com> (raw)
In-Reply-To: <4d18d015-4154-5a0c-e93d-16b8bdbdaddb@nvidia.com>
On 09/12/2021 18:08, Nikolay Aleksandrov wrote:
> On 09/12/2021 17:42, Jakub Kicinski wrote:
>> On Thu, 9 Dec 2021 13:14:32 +0100 David Lamparter wrote:
>>> Split-horizon essentially just means being able to create multiple
>>> groups of isolated ports that are isolated within the group, but not
>>> with respect to each other.
>>>
>>> The intent is very different, while isolation is a policy feature,
>>> split-horizon is intended to provide functional "multiple member ports
>>> are treated as one for loop avoidance." But it boils down to the same
>>> thing in the end.
>>>
>>> Signed-off-by: David Lamparter <equinox@diac24.net>
>>> Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
>>> Cc: Alexandra Winter <wintera@linux.ibm.com>
>>
>> Does not apply to net-next, you'll need to repost even if the code is
>> good. Please put [PATCH net-next] in the subject.
>>
>
> Hi,
> For some reason this patch didn't make it to my inbox.. Anyway I was
> able to see it now online, a few comments (sorry can't do them inline due
> to missing mbox patch):
> - please drop the sysfs part, we're not extending sysfs anymore
> - split the bridge change from the driver
> - drop the /* BR_ISOLATED - previously BIT(16) */ comment
> - [IFLA_BRPORT_HORIZON_GROUP] = NLA_POLICY_MIN(NLA_S32, 0), why not just { .type = NLA_U32 } ?
> - just forbid having both set (tb[IFLA_BRPORT_ISOLATED] && tb[IFLA_BRPORT_HORIZON_GROUP])
> user-space should use just one of the two, if isolated is set then it overwrites any older
> IFLA_BRPORT_HORIZON_GROUP settings, that should simplify things considerably
Actually they'll have to be exported together always... that's unfortunate. I get
why you need the extra netlink logic, I think we'll have to keep it.
next prev parent reply other threads:[~2021-12-09 16:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 12:14 [PATCH] bridge: extend BR_ISOLATE to full split-horizon David Lamparter
2021-12-09 15:42 ` Jakub Kicinski
2021-12-09 16:08 ` Nikolay Aleksandrov
2021-12-09 16:23 ` Nikolay Aleksandrov [this message]
2021-12-09 16:57 ` Nikolay Aleksandrov
2021-12-10 16:49 ` David Lamparter
2021-12-09 16:23 ` Alexandra Winter
2021-12-10 16:53 ` Alexandra Winter
2021-12-10 16:57 ` David Lamparter
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=481128e0-d4d0-3fde-6a9e-65e391bbf64d@nvidia.com \
--to=nikolay@nvidia.com \
--cc=equinox@diac24.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=wintera@linux.ibm.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).