* [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
@ 2014-12-29 21:05 roopa
2014-12-29 23:42 ` Scott Feldman
0 siblings, 1 reply; 5+ messages in thread
From: roopa @ 2014-12-29 21:05 UTC (permalink / raw)
To: netdev, shemminger, vyasevic; +Cc: Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks•com>
vlan add/deletes are not notified to userspace today. This patch fixes it.
Notifications will contain vlans compressed into ranges whereever applicable.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks•com>
---
net/bridge/br_netlink.c | 3 ++-
net/core/rtnetlink.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 16bdd5a..cebfb03 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -303,7 +303,8 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port)
if (skb == NULL)
goto errout;
- err = br_fill_ifinfo(skb, port, 0, 0, event, 0, 0, port->dev);
+ err = br_fill_ifinfo(skb, port, 0, 0, event, 0,
+ RTEXT_FILTER_BRVLAN_COMPRESSED, port->dev);
if (err < 0) {
/* -EMSGSIZE implies BUG in br_nlmsg_size() */
WARN_ON(err == -EMSGSIZE);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d06107d..dad5fb6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2878,7 +2878,8 @@ static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) &&
br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
- err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
+ err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev,
+ RTEXT_FILTER_BRVLAN_COMPRESSED);
if (err < 0)
goto errout;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
2014-12-29 21:05 [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages roopa
@ 2014-12-29 23:42 ` Scott Feldman
2014-12-30 5:17 ` roopa
0 siblings, 1 reply; 5+ messages in thread
From: Scott Feldman @ 2014-12-29 23:42 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Netdev, shemminger, vyasevic@redhat•com
On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks•com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks•com>
>
> vlan add/deletes are not notified to userspace today. This patch fixes it.
> Notifications will contain vlans compressed into ranges whereever applicable.
Why is this notification needed? On VLAN add/del, the port driver
will already get called if it implements ndo_vlan_rx_add_vid and
ndo_vlan_rx_kill_vid. The port driver already gets the notification
it needs to filter vlans. User space can query for vlan port
membership if it needs to know.
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks•com>
> ---
> net/bridge/br_netlink.c | 3 ++-
> net/core/rtnetlink.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 16bdd5a..cebfb03 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -303,7 +303,8 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port)
> if (skb == NULL)
> goto errout;
>
> - err = br_fill_ifinfo(skb, port, 0, 0, event, 0, 0, port->dev);
> + err = br_fill_ifinfo(skb, port, 0, 0, event, 0,
> + RTEXT_FILTER_BRVLAN_COMPRESSED, port->dev);
This doesn't look right. The skb wasn't allocated large enough to
hold vlan info. If this is working in your tests, you're getting
lucky due to some skb padding.
skb was allocated with br_nlmsg_size(), which doesn't include vlan info.
> if (err < 0) {
> /* -EMSGSIZE implies BUG in br_nlmsg_size() */
> WARN_ON(err == -EMSGSIZE);
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index d06107d..dad5fb6 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2878,7 +2878,8 @@ static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
>
> if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) &&
> br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
> - err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
> + err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev,
> + RTEXT_FILTER_BRVLAN_COMPRESSED);
> if (err < 0)
> goto errout;
> }
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger•kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
2014-12-29 23:42 ` Scott Feldman
@ 2014-12-30 5:17 ` roopa
2014-12-30 5:25 ` Scott Feldman
0 siblings, 1 reply; 5+ messages in thread
From: roopa @ 2014-12-30 5:17 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat•com, Wilson Kok
On 12/29/14, 3:42 PM, Scott Feldman wrote:
> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks•com> wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks•com>
>>
>> vlan add/deletes are not notified to userspace today. This patch fixes it.
>> Notifications will contain vlans compressed into ranges whereever applicable.
> Why is this notification needed? On VLAN add/del, the port driver
> will already get called if it implements ndo_vlan_rx_add_vid and
> ndo_vlan_rx_kill_vid.
This is strictly for userspace.
> The port driver already gets the notification
> it needs to filter vlans. User space can query for vlan port
> membership if it needs to know.
You mean request a dump ?. yes it can if it needs all the configured vlans.
But for notifications, we must at the least notify what changed with
respect to vlans with RTM_SETLINK/DELLINK, no ?
>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks•com>
>> ---
>> net/bridge/br_netlink.c | 3 ++-
>> net/core/rtnetlink.c | 3 ++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 16bdd5a..cebfb03 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -303,7 +303,8 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port)
>> if (skb == NULL)
>> goto errout;
>>
>> - err = br_fill_ifinfo(skb, port, 0, 0, event, 0, 0, port->dev);
>> + err = br_fill_ifinfo(skb, port, 0, 0, event, 0,
>> + RTEXT_FILTER_BRVLAN_COMPRESSED, port->dev);
> This doesn't look right. The skb wasn't allocated large enough to
> hold vlan info. If this is working in your tests, you're getting
> lucky due to some skb padding.
>
> skb was allocated with br_nlmsg_size(), which doesn't include vlan info.
>
>> if (err < 0) {
>> /* -EMSGSIZE implies BUG in br_nlmsg_size() */
>> WARN_ON(err == -EMSGSIZE);
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index d06107d..dad5fb6 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -2878,7 +2878,8 @@ static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
>>
>> if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) &&
>> br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
>> - err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
>> + err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev,
>> + RTEXT_FILTER_BRVLAN_COMPRESSED);
>> if (err < 0)
>> goto errout;
>> }
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger•kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
2014-12-30 5:17 ` roopa
@ 2014-12-30 5:25 ` Scott Feldman
2014-12-30 5:41 ` roopa
0 siblings, 1 reply; 5+ messages in thread
From: Scott Feldman @ 2014-12-30 5:25 UTC (permalink / raw)
To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat•com, Wilson Kok
On Mon, Dec 29, 2014 at 9:17 PM, roopa <roopa@cumulusnetworks•com> wrote:
> On 12/29/14, 3:42 PM, Scott Feldman wrote:
>>
>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks•com> wrote:
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks•com>
>>>
>>> vlan add/deletes are not notified to userspace today. This patch fixes
>>> it.
>>> Notifications will contain vlans compressed into ranges whereever
>>> applicable.
>>
>> Why is this notification needed? On VLAN add/del, the port driver
>> will already get called if it implements ndo_vlan_rx_add_vid and
>> ndo_vlan_rx_kill_vid.
>
> This is strictly for userspace.
>
>> The port driver already gets the notification
>> it needs to filter vlans. User space can query for vlan port
>> membership if it needs to know.
>
>
> You mean request a dump ?. yes it can if it needs all the configured vlans.
> But for notifications, we must at the least notify what changed with respect
> to vlans with RTM_SETLINK/DELLINK, no ?
You'll get that notification if you implement ndo_vlan_rx_add/kill_vid
in your port driver, which is called from RTM_SETLINK/DELLINK. Your
port driver can send to user-space, I suppose, if user-space needs
notification.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
2014-12-30 5:25 ` Scott Feldman
@ 2014-12-30 5:41 ` roopa
0 siblings, 0 replies; 5+ messages in thread
From: roopa @ 2014-12-30 5:41 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat•com, Wilson Kok
On 12/29/14, 9:25 PM, Scott Feldman wrote:
> On Mon, Dec 29, 2014 at 9:17 PM, roopa <roopa@cumulusnetworks•com> wrote:
>> On 12/29/14, 3:42 PM, Scott Feldman wrote:
>>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks•com> wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks•com>
>>>>
>>>> vlan add/deletes are not notified to userspace today. This patch fixes
>>>> it.
>>>> Notifications will contain vlans compressed into ranges whereever
>>>> applicable.
>>> Why is this notification needed? On VLAN add/del, the port driver
>>> will already get called if it implements ndo_vlan_rx_add_vid and
>>> ndo_vlan_rx_kill_vid.
>> This is strictly for userspace.
>>
>>> The port driver already gets the notification
>>> it needs to filter vlans. User space can query for vlan port
>>> membership if it needs to know.
>>
>> You mean request a dump ?. yes it can if it needs all the configured vlans.
>> But for notifications, we must at the least notify what changed with respect
>> to vlans with RTM_SETLINK/DELLINK, no ?
> You'll get that notification if you implement ndo_vlan_rx_add/kill_vid
> in your port driver, which is called from RTM_SETLINK/DELLINK. Your
> port driver can send to user-space, I suppose, if user-space needs
> notification.
When the bridge is a vlan filtering bridge, I think its the bridge
driver who will need to send notifications to vlan add/dels
on PF_BRIDGE similar to fdb add/dels.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-30 5:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-29 21:05 [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages roopa
2014-12-29 23:42 ` Scott Feldman
2014-12-30 5:17 ` roopa
2014-12-30 5:25 ` Scott Feldman
2014-12-30 5:41 ` roopa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox