All of lore.kernel.org
 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 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.