netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: "Arad, Ronen" <ronen.arad@intel.com>
Cc: "Roopa Prabhu" <roopa@cumulusnetworks.com>,
	"Scott Feldman" <sfeldma@gmail.com>,
	Netdev <netdev@vger.kernel.org>, "Jirí Pírko" <jiri@resnulli.us>,
	"Jamal Hadi Salim" <jhs@mojatatu.com>,
	"Benjamin LaHaise" <bcrl@kvack.org>,
	"Thomas Graf" <tgraf@suug.ch>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"John Linville" <linville@tuxdriver.com>,
	"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
	"Nicolas Dichtel" <nicolas.dichtel@6wind.com>,
	"vyasevic@redhat.com" <vyasevic@redhat.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"buytenh@wantstofly.org" <buytenh@wantstofly.org>,
	"Aviad Raveh" <aviadr@mellanox.com>,
	"David S. Miller" <davem@davemloft.net>,
	"shm@cumulusnetworks.com" <shm@cumulusnetworks.com>,
	"Andy Gospodarek" <gospo@cumulusnetworks.com>
Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
Date: Fri, 05 Dec 2014 18:46:23 -0800	[thread overview]
Message-ID: <54826DFF.6090906@gmail.com> (raw)
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505D8436D@ORSMSX101.amr.corp.intel.com>

On 12/05/2014 05:04 PM, Arad, Ronen wrote:
> I have another case of propagation which is not covered by the proposed patch.
> A recent patch introduced default_pvid attribute for a bridge (so far supported only via sysfs and not via netlink).
> When a port joins a bridge, it inherits a PVID from the default_pvid of the bridge.
> The bridge driver propagates that to the newly created net_bridge_port. This is done in br_vlan.c:
>
> int nbp_vlan_init(struct net_bridge_port *p)
> {
> 	int rc = 0;
>
> 	if (p->br->default_pvid) {
> 		rc = nbp_vlan_add(p, p->br->default_pvid,
> 				  BRIDGE_VLAN_INFO_PVID |
> 				  BRIDGE_VLAN_INFO_UNTAGGED);
> 	}
>
> 	return rc;
> }
>
> When L2 switching is offloaded to the HW, this PVID setting need to be propagated. However, it does not come via ndo_bridge_setlink. The proposed propagation at br_setlink or an up level one at rtnetlink are not capable of handling this case.
> One possible way for handling that is to replace the call to nbp_vlan_add with a call to a new function let's say
> int br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
> This function will compose a netlink message with VLAN filtering information (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload support proposed by Roopa.
>

No, we shouldn't be crafting netlink messages in the kernel just
re-inject them into an interface. Really the setlink/dellink interface
should be cleaned up so that it no longer consumes raw netlink messages.

Then either (a) add another parameter to the setlink ops or (b) create
a new op for it.

I think cleaning up the setlink/dellink hooks is on the TBD list
already.


> If this is an acceptable course of action, I could work on such patch.
>
>

[...]

Thanks,
John

-- 
John Fastabend         Intel Corporation

  reply	other threads:[~2014-12-06  2:46 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-05  2:26 [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set roopa
2014-12-05  6:41 ` Scott Feldman
2014-12-05  6:55   ` John Fastabend
2014-12-05  7:10     ` Roopa Prabhu
2014-12-05 12:41       ` Jamal Hadi Salim
2014-12-05 14:03         ` Roopa Prabhu
2014-12-05  7:02   ` Roopa Prabhu
2014-12-05 23:21     ` Arad, Ronen
2014-12-06  1:04       ` Arad, Ronen
2014-12-06  2:46         ` John Fastabend [this message]
2014-12-06  3:06           ` Arad, Ronen
2014-12-06  3:21             ` John Fastabend
2014-12-06  6:29         ` Scott Feldman
2014-12-06  8:05           ` Arad, Ronen
2014-12-07 17:33             ` Roopa Prabhu
2014-12-06  6:54       ` Scott Feldman
2014-12-07 20:24         ` Roopa Prabhu
2014-12-08  4:56           ` Roopa Prabhu
2014-12-08 11:14           ` Jiri Pirko
2014-12-08 18:40           ` Arad, Ronen
2014-12-07 19:13       ` Roopa Prabhu
2014-12-05  7:38 ` Jiri Pirko
2014-12-05 13:44   ` Roopa Prabhu
2014-12-05 14:37   ` Roopa Prabhu
2014-12-05 12:07 ` Jamal Hadi Salim
2014-12-05 13:21 ` Sergei Shtylyov
2014-12-05 14:04   ` Roopa Prabhu

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=54826DFF.6090906@gmail.com \
    --to=john.fastabend@gmail.com \
    --cc=aviadr@mellanox.com \
    --cc=bcrl@kvack.org \
    --cc=buytenh@wantstofly.org \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=gospo@cumulusnetworks.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=nicolas.dichtel@6wind.com \
    --cc=ronen.arad@intel.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=sfeldma@gmail.com \
    --cc=shm@cumulusnetworks.com \
    --cc=stephen@networkplumber.org \
    --cc=tgraf@suug.ch \
    --cc=vyasevic@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 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).