* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Roopa Prabhu @ 2014-12-11 16:37 UTC (permalink / raw)
To: Jiri Pirko
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141211110115.GA1979@nanopsycho.lan>
On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>> -----Original Message-----
>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>> To: Jiri Pirko
>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>> stephen@networkplumber.org; Fastabend, John R;
>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>> configuration
>>>
>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>> From: Marco Varlese <marco.varlese@intel.com>
>>>>>
>>>>> Switch hardware offers a list of attributes that are configurable on
>>>>> a per port basis.
>>>>> This patch provides a mechanism to configure switch ports by adding
>>>>> an NDO for setting specific values to specific attributes.
>>>>> There will be a separate patch that extends iproute2 to call the new
>>>>> NDO.
>>>>
>>>> What are these attributes? Can you give some examples. I'm asking
>>>> because there is a plan to pass generic attributes to switch ports
>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>> bridge is setting that attribute.
>>>>
>>>> Is there need to set something directly from userspace or does it make
>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>> be needed.
>>> +1
>>>
>>> I think for many attributes it would be best to have both. The in kernel callers
>>> and netlink userspace can use the same driver ndo_ops.
>>>
>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>> may have some attributes that are not specific to any existing software
>>> module. I'm guessing Marco has some examples of these.
>>>
>>> [...]
>>>
>>>
>>> --
>>> John Fastabend Intel Corporation
>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>
>> An example of attributes are:
>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>
>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>
>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>
>> I hope this clarifies some points.
> It does. Makes sense. We need to expose this attr set/get for both
> in-kernel and userspace use cases.
>
> Please adjust you patch for this. Also, as a second patch, it would be
> great if you can convert ndo_switch_port_stp_update to this new ndo.
Why are we exposing generic switch attribute get/set from userspace ?.
We already have specific attributes for learning/flooding which can be
extended further.
And for in kernel api....i had a sample patch in my RFC series (Which i
was going to resubmit, until it was decided that we will use existing
api around ndo_bridge_setlink/ndo_bridge_getlink):
http://www.spinics.net/lists/netdev/msg305473.html
Thanks,
Roopa
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 16:39 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, Jamal Hadi Salim
In-Reply-To: <54894850.5000603@cumulusnetworks.com>
> I think commit
> "5e6d243587990a588143b9da3974833649595587 "bridge: netlink dump
> interface at par with brctl" tried to make sure even the dflt entries
> (ie dev->uc and dev->mc) were also printed in the below case. ie the
> 'self' entries in the below output.
>
> # ./bridge fdb show brport eth1
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
>
> Am guessing reverting the patch is going to make the 'self' entries in
> the above output to go away.
> Can you please confirm ?.
I don't want you to revert the patch, as the main goal of the patch
was to enable filtering in the kernel. I am only proposing
to revert part of it that allows driver to implement own dump.
This does not break the filtering in the kernel.
Whether the 'self' entries will go away it depends if the driver
overrides ndo_fdb_dump callback with its own. For cases where the driver
does not implement the callback, the dflt callback is still called
showing 'self' entries:
[root@silpixa00378825 ~]# bridge fdb show
33:33:00:00:00:01 dev em1 self permanent
01:00:5e:00:00:01 dev em1 self permanent
33:33:00:00:00:01 dev p4p1 self permanent
01:00:5e:00:00:01 dev p4p1 self permanent
33:33:ff:81:56:db dev p4p1 self permanent
01:00:5e:00:00:fb dev p4p1 self permanent
33:33:00:00:00:01 dev dummy0 self permanent
>
> Also, if i hear your concern correctly, for bridge ports that implement
> ndo_fdb_dump, with commit 5e6d243587990a588143b9da3974833649595587, we
> will get two entries for each 'self' entry above.
> Can you also paste sample output for that ?.
My patch affects *only* drivers that implements own dump callback.
Implementing own dump callback means the driver want to have a control
over what is being dumped. For example you may want to dump a hardware
MAC table only (my case) where 'self' entries created by kernel make no sense.
Also there are drivers that calls dflt callback from inside own dump function.
Please see following dump callback implemented for QLogic:
static int qlcnic_fdb_dump(struct sk_buff *skb, struct netlink_callback *ncb,
struct net_device *netdev,
struct net_device *filter_dev, int idx)
{
struct qlcnic_adapter *adapter = netdev_priv(netdev);
if (!adapter->fdb_mac_learn)
return ndo_dflt_fdb_dump(skb, ncb, netdev, filter_dev, idx);
[...]
Another example of dflt being called twice is macvlan.c where ndo_fdb_dump
is actually initialized with the dflt callback:
macvlan.c:1022: .ndo_fdb_dump = ndo_dflt_fdb_dump,
Thanks,
Hubert
^ permalink raw reply
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: vadim4j @ 2014-12-11 16:33 UTC (permalink / raw)
To: Jiri Benc; +Cc: Vadim Kochan, netdev
In-Reply-To: <20141211170928.62c8e637@griffin>
On Thu, Dec 11, 2014 at 05:09:28PM +0100, Jiri Benc wrote:
> On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote:
> > From: Vadim Kochan <vadim4j@gmail.com>
> >
> > Added new '-ns' option to simplify executing following cmd:
> >
> > ip netns exec NETNS ip OPTIONS COMMAND OBJECT
> >
> > to
> >
> > ip -ns NETNS OPTIONS COMMAND OBJECT
> >
> > e.g.:
> >
> > ip -ns vnet0 link add br0 type bridge
>
> This is great! It's a thing that has been bothering me for long time
> but never got high enough on my todo list. Thanks for working on this.
>
> However,
>
> > --- a/ip/ip.c
> > +++ b/ip/ip.c
> > @@ -262,6 +262,12 @@ int main(int argc, char **argv)
> > rcvbuf = size;
> > } else if (matches(opt, "-help") == 0) {
> > usage();
> > + } else if (matches(opt, "-ns") == 0) {
> > + argc--;
> > + argv++;
> > + argv[0] = argv[1];
> > + argv[1] = basename;
> > + return netns_exec(argc, argv);
>
> I very much dislike this. There's no reason to exec another ip binary.
> The main reason I wanted the -n (or whatever) option was to speed up
> execution of test scripts in environments with hundreds of interfaces
> in different net namespaces.
>
> Please just change to the specified netns and continue with interpreting
> of the rest of the command line, there's absolutely no reason for doing
> the exec.
Yes, I will follow that way.
Thanks,
>
> Thanks,
>
> Jiri
>
> --
> Jiri Benc
^ permalink raw reply
* Re: tg3 broken in 3.18.0?
From: Marcelo Ricardo Leitner @ 2014-12-11 16:45 UTC (permalink / raw)
To: Nils Holland, netdev
In-Reply-To: <20141210230634.GA22884@teela.fritz.box>
On 10-12-2014 21:06, Nils Holland wrote:
> Hi everyone,
>
> I just upgraded a machine from 3.17.3 to 3.18.0 and noticed that after
> the upgrade, the machine's network interface (which is a tg3) would no
> longer run correctly (or, for that matter, run at all). During the
> upgrade, I didn't change any kernel config options or any other parts
> of the system.
Same thing here! Thanks for reporting this, Nils.
> Since the machine is remote and I don't have direct access to it, it's
> kind of hard currently to give more details, but here's what I'm
> seeing in the logs:
I have access to mine, kudos to secondary NIC.
$ ethtool -i p1p1
driver: tg3
version: 3.137
firmware-version: 5722-v3.13
bus-info: 0000:02:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no
$ ethtool p1p1
Settings for p1p1:
Supported ports: [ TP ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Speed: Unknown!
Duplex: Unknown! (255)
Port: Twisted Pair
PHYAD: 1
Transceiver: internal
Auto-negotiation: on
MDI-X: Unknown
$ sudo ip link set p1p1 up
RTNETLINK answers: No such device
> If I see things correctly, there were only two patches affecting tg3
> between 3.17(.3) and 3.18:
>
> 2c7c9ea429ba30fe506747b7da110e2212d8fefa
> a620a6bc1c94c22d6c312892be1e0ae171523125
I'm running net-next, 395eea6ccf2b253f81b4718ffbcae67d36fe2e69.
So my diffs would be:
$ git log v3.17..origin/master --oneline -- drivers/net/ethernet/broadcom/tg3.c
892311f ethtool: Support for configurable RSS hash function
60b7379 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
a620a6b tg3: fix ring init when there are more TX than RX channels
3964835 tg3: use netdev_rss_key_fill() helper
2c7c9ea tg3: Add skb->xmit_more support
Reverting all these, issue continues.
If no one has a better shot, I'll try bissecting later.
Marcelo
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 16:51 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David Miller, netdev, Roopa Prabhu, Vlad Yasevich
In-Reply-To: <548984C0.2040706@mojatatu.com>
>
> It wont work. As pointed out by Roopa in
> the other email dev->uc/mc will not get dumped with this
> change. Vlad will be in a better position to comment.
> CCing Vlad.
Jamal, this will still get dumped unless your driver implements
own dump callback. Can you please exactly tell us what wont work?
Please see my previous post where I added more explanation.
Thanks,
Hubert
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Roopa Prabhu @ 2014-12-11 16:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <20141210093755.GB1863@nanopsycho.orion>
On 12/10/14, 1:37 AM, Jiri Pirko wrote:
> Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds two new api's netdev_switch_port_bridge_setlink
>> and netdev_switch_port_bridge_dellink to offload bridge port attributes
>> to switch asic
>>
>> (The names of the apis look odd with 'switch_port_bridge',
>> but am more inclined to change the prefix of the api to something else.
>> Will take any suggestions).
>>
>> The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>> pass bridge port attributes to the port device.
>>
>> If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>> the bridge port attribute offload ndo, call bridge port attribute ndo's on
>> the lowerdevs if supported. This is one way to pass bridge port attributes
>> through stacked netdevs (example when bridge port is a bond and bond slaves
>> are switch ports).
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/net/switchdev.h | 5 +++-
>> net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 74 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index 8a6d164..22676b6 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -17,7 +17,10 @@
>> int netdev_switch_parent_id_get(struct net_device *dev,
>> struct netdev_phys_item_id *psid);
>> int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>> -
>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags);
>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags);
>> #else
>>
>> static inline int netdev_switch_parent_id_get(struct net_device *dev,
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index d162b21..62317e1 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>> return ops->ndo_switch_port_stp_update(dev, state);
>> }
>> EXPORT_SYMBOL(netdev_switch_port_stp_update);
>> +
>> +/**
>> + * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>> + * port attributes
>> + *
>> + * @dev: port device
>> + * @nlh: netlink msg with bridge port attributes
>> + *
>> + * Notify switch device port of bridge port attributes
>> + */
>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags)
>> +{
>> + const struct net_device_ops *ops = dev->netdev_ops;
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> + int ret = 0, err = 0;
>> +
>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>> + return err;
>> +
>> + if (ops->ndo_bridge_setlink) {
>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>> + return ops->ndo_bridge_setlink(dev, nlh, flags);
> You have to change ndo_bridge_setlink in netdevice.h first.
> Otherwise when only this patch is applied (during bisection)
> this won't compile.
ack, will fix it and keep that in mind next time.
>
>> + }
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
> I do not understand why to iterate over lower devices. At this
> stage we don't know a thing about this upper or its lowers. Let
> the uppers (/masters) to decide if this needs to be propagated
> or not.
Jiri, In the stacked devices case, there is no way to propagate the
bridge port attributes to switch device driver today (vlan and other
bridge port attributes). Can you tell me if there is a way ?. no,
ndo_vlan* ndo's are not useful here. Nor we should go and implement
ndo_bridge_setlink* in all devices that can be bridge ports.
And this allows a switch driver to receive these callbacks if it has
marked the switch port with an offload flag. Your way of using the
switch port to get to the switch driver does not help in these cases.
The other option is to use the 'switch device (not port)' to get to the
switch driver.
This patch shows that you can still do this with the ndo ops.
>
>> + err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>> + if (err)
>> + ret = err;
>> + }
> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>> +
>> +/**
>> + * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>> + * attribute delete
>> + *
>> + * @dev: port device
>> + * @nlh: netlink msg with bridge port attributes
>> + *
>> + * Notify switch device port of bridge port attribute delete
>> + */
>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags)
>> +{
>> + const struct net_device_ops *ops = dev->netdev_ops;
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> + int ret = 0, err = 0;
>> +
>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>> + return err;
>> +
>> + if (ops->ndo_bridge_dellink) {
>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>> + return ops->ndo_bridge_dellink(dev, nlh, flags);
>> + }
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>> + err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>> + if (err)
>> + ret = err;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>> --
>> 1.7.10.4
>>
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-11 16:56 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <5489C85A.5020400@cumulusnetworks.com>
Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>-----Original Message-----
>>>>From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>Sent: Wednesday, December 10, 2014 5:04 PM
>>>>To: Jiri Pirko
>>>>Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>stephen@networkplumber.org; Fastabend, John R;
>>>>roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>kernel@vger.kernel.org
>>>>Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>configuration
>>>>
>>>>On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>>>From: Marco Varlese <marco.varlese@intel.com>
>>>>>>
>>>>>>Switch hardware offers a list of attributes that are configurable on
>>>>>>a per port basis.
>>>>>>This patch provides a mechanism to configure switch ports by adding
>>>>>>an NDO for setting specific values to specific attributes.
>>>>>>There will be a separate patch that extends iproute2 to call the new
>>>>>>NDO.
>>>>>
>>>>>What are these attributes? Can you give some examples. I'm asking
>>>>>because there is a plan to pass generic attributes to switch ports
>>>>>replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>bridge is setting that attribute.
>>>>>
>>>>>Is there need to set something directly from userspace or does it make
>>>>>rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>be needed.
>>>>+1
>>>>
>>>>I think for many attributes it would be best to have both. The in kernel callers
>>>>and netlink userspace can use the same driver ndo_ops.
>>>>
>>>>But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>may have some attributes that are not specific to any existing software
>>>>module. I'm guessing Marco has some examples of these.
>>>>
>>>>[...]
>>>>
>>>>
>>>>--
>>>>John Fastabend Intel Corporation
>>>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>
>>>An example of attributes are:
>>>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>
>>>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>
>>>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>
>>>I hope this clarifies some points.
>>It does. Makes sense. We need to expose this attr set/get for both
>>in-kernel and userspace use cases.
>>
>>Please adjust you patch for this. Also, as a second patch, it would be
>>great if you can convert ndo_switch_port_stp_update to this new ndo.
>
>Why are we exposing generic switch attribute get/set from userspace ?. We
>already have specific attributes for learning/flooding which can be extended
>further.
Yes, but that is for PF_BRIDGE and bridge specific attributes. There
might be another generic attrs, no?
>And for in kernel api....i had a sample patch in my RFC series (Which i was
>going to resubmit, until it was decided that we will use existing api around
>ndo_bridge_setlink/ndo_bridge_getlink):
>http://www.spinics.net/lists/netdev/msg305473.html
Yes, this might become handy for other generic non-bridge attrs.
>
>Thanks,
>Roopa
>
>
>
^ permalink raw reply
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: David Miller @ 2014-12-11 17:01 UTC (permalink / raw)
To: m-karicheri2
Cc: netdev, linux-arm-kernel, linux-kernel, devicetree, robh+dt,
grant.likely
In-Reply-To: <5489A6CD.20909@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Thu, 11 Dec 2014 09:14:37 -0500
> BTW, could you provide any suggestions that would help us merge this
> series to upstream? This has been sitting on this list for a while
> now.
You simply have to continue going through the review and revision
process until people no longer find problems with your changes.
This could take several more weeks, you simply must be patient.
^ permalink raw reply
* Re: [PATCH net-next RESEND 2/2] ipv6: fix sparse warning
From: Paul E. McKenney @ 2014-12-11 17:02 UTC (permalink / raw)
To: Ying Xue
Cc: davem, eric.dumazet, jon.maloy, erik.hugne, netdev, kbuild-all,
linux-kernel
In-Reply-To: <5488F9D1.9060109@windriver.com>
On Thu, Dec 11, 2014 at 09:56:33AM +0800, Ying Xue wrote:
> On 12/11/2014 12:04 AM, Paul E. McKenney wrote:
> > On Wed, Dec 10, 2014 at 04:46:07PM +0800, Ying Xue wrote:
> >> This fixes the following spare warning when using
> >>
> >> make C=1 CF=-D__CHECK_ENDIAN__ net/ipv6/addrconf.o
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >>
> >> To silence above spare complaint, an RCU annotation should be added
> >> to "next" pointer of hlist_node structure through hlist_next_rcu()
> >> macro when iterating over a hlist with
> >> hlist_for_each_entry_continue_rcu_bh().
> >>
> >> By the way, this commit also resolves the same error appearing in
> >> hlist_for_each_entry_continue_rcu().
> >>
> >> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> >
> > If you pull the rculist.h changes from the first patch into this one,
> > I will queue it up through -rcu.
>
> Please just queue this patch into your RCU tree. Some changes of the
> first patch currently only exists in net-next tree, so it cannot be
> merged into RCU tree now. Therefore, please temporarily ignore it. In
> next 3.19 development cycle, I will resubmit it to you.
Sounds good, but could you please rebase the second patch to be
independent of the first patch? If you do that, I would be happy
to queue it.
Thanx, Paul
> Thanks,
> Ying
>
> > Thanx, Paul
> >
> >> ---
> >> include/linux/rculist.h | 16 ++++++++--------
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> >> index 866d9c9..32bd4ad 100644
> >> --- a/include/linux/rculist.h
> >> +++ b/include/linux/rculist.h
> >> @@ -524,11 +524,11 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> >> * @member: the name of the hlist_node within the struct.
> >> */
> >> #define hlist_for_each_entry_continue_rcu(pos, member) \
> >> - for (pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
> >> - typeof(*(pos)), member); \
> >> + for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member); \
> >> pos; \
> >> - pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
> >> - typeof(*(pos)), member))
> >> + pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member))
> >>
> >> /**
> >> * hlist_for_each_entry_continue_rcu_bh - iterate over a hlist continuing after current point
> >> @@ -536,11 +536,11 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> >> * @member: the name of the hlist_node within the struct.
> >> */
> >> #define hlist_for_each_entry_continue_rcu_bh(pos, member) \
> >> - for (pos = hlist_entry_safe(rcu_dereference_bh((pos)->member.next),\
> >> - typeof(*(pos)), member); \
> >> + for (pos = hlist_entry_safe(rcu_dereference_bh(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member); \
> >> pos; \
> >> - pos = hlist_entry_safe(rcu_dereference_bh((pos)->member.next),\
> >> - typeof(*(pos)), member))
> >> + pos = hlist_entry_safe(rcu_dereference_bh(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member))
> >>
> >> /**
> >> * hlist_for_each_entry_from_rcu - iterate over a hlist continuing from current point
> >> --
> >> 1.7.9.5
> >>
> >
> >
> >
>
^ permalink raw reply
* Re: [PATCH net-next v9 0/3] add hisilicon hip04 ethernet driver
From: David Miller @ 2014-12-11 17:04 UTC (permalink / raw)
To: dingtianhong-hv44wF8Li93QT0dZR+AlfA
Cc: zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
arnd-r2nGTMty4D4, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
mark.rutland-5wv7dgnIgG8, David.Laight-ZS65k/vG3HxXrIkS9f7CXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1418298150-4944-1-git-send-email-dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
The net-next tree is closed, therefore it is not appropriate to
submit net-next changes at this time.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Driver: Vmxnet3: Make Rx ring 2 size configurable
From: David Miller @ 2014-12-11 17:05 UTC (permalink / raw)
To: skhare; +Cc: sbhatewara, pv-drivers, netdev, linux-kernel, bollar
In-Reply-To: <1418276262-4728-1-git-send-email-skhare@vmware.com>
From: Shrikrishna Khare <skhare@vmware.com>
Date: Wed, 10 Dec 2014 21:37:42 -0800
> Rx ring 2 size can be configured by adjusting rx-jumbo parameter
> of ethtool -G.
>
> Signed-off-by: Ramya Bolla <bollar@vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
This is a new feature patch, and the net-next tree is closed, therefore
it is not appropriate to submit a change like this at this time.
Please resubmit when the net-next tree opens back up, some time after
the merge window is over.
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 17:06 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, Jamal Hadi Salim
In-Reply-To: <54894850.5000603@cumulusnetworks.com>
My apologies for sending again, I forgot to include a sample output you asked.
> Also, if i hear your concern correctly, for bridge ports that implement
> ndo_fdb_dump, with commit 5e6d243587990a588143b9da3974833649595587, we
> will get two entries for each 'self' entry above.
> Can you also paste sample output for that ?.
>
[root@silpixa00378825 ~]# bridge fdb show brport mac0
33:33:00:00:00:01 self permanent
33:33:00:00:00:01 self permanent
Regards,
Hubert
^ permalink raw reply
* Re: [PATCH v2 1/4 net-next] net: bcmgenet: bcmgenet_init_tx_ring() cleanup
From: David Miller @ 2014-12-11 17:09 UTC (permalink / raw)
To: pgynther; +Cc: netdev, f.fainelli
In-Reply-To: <20141211102030.5E534222AAA@puck.mtv.corp.google.com>
The net-next tree is closed, as I announced last evening, therefore
submitting changes for that tree right now is not appropriate.
Please resubmit this series when the net-next tree opens back up,
and also please when you do so provide a proper cover "0/N" posting
for your patch series.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Jiri Pirko @ 2014-12-11 17:11 UTC (permalink / raw)
To: Roopa Prabhu
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <5489CBBA.7050302@cumulusnetworks.com>
Thu, Dec 11, 2014 at 05:52:10PM CET, roopa@cumulusnetworks.com wrote:
>On 12/10/14, 1:37 AM, Jiri Pirko wrote:
>>Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>>This patch adds two new api's netdev_switch_port_bridge_setlink
>>>and netdev_switch_port_bridge_dellink to offload bridge port attributes
>>>to switch asic
>>>
>>>(The names of the apis look odd with 'switch_port_bridge',
>>>but am more inclined to change the prefix of the api to something else.
>>>Will take any suggestions).
>>>
>>>The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>>>pass bridge port attributes to the port device.
>>>
>>>If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>>>the bridge port attribute offload ndo, call bridge port attribute ndo's on
>>>the lowerdevs if supported. This is one way to pass bridge port attributes
>>>through stacked netdevs (example when bridge port is a bond and bond slaves
>>>are switch ports).
>>>
>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>---
>>>include/net/switchdev.h | 5 +++-
>>>net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>2 files changed, 74 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>index 8a6d164..22676b6 100644
>>>--- a/include/net/switchdev.h
>>>+++ b/include/net/switchdev.h
>>>@@ -17,7 +17,10 @@
>>>int netdev_switch_parent_id_get(struct net_device *dev,
>>> struct netdev_phys_item_id *psid);
>>>int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>-
>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags);
>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags);
>>>#else
>>>
>>>static inline int netdev_switch_parent_id_get(struct net_device *dev,
>>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>index d162b21..62317e1 100644
>>>--- a/net/switchdev/switchdev.c
>>>+++ b/net/switchdev/switchdev.c
>>>@@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>> return ops->ndo_switch_port_stp_update(dev, state);
>>>}
>>>EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>+
>>>+/**
>>>+ * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>>>+ * port attributes
>>>+ *
>>>+ * @dev: port device
>>>+ * @nlh: netlink msg with bridge port attributes
>>>+ *
>>>+ * Notify switch device port of bridge port attributes
>>>+ */
>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags)
>>>+{
>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>+ struct net_device *lower_dev;
>>>+ struct list_head *iter;
>>>+ int ret = 0, err = 0;
>>>+
>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>+ return err;
>>>+
>>>+ if (ops->ndo_bridge_setlink) {
>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>+ return ops->ndo_bridge_setlink(dev, nlh, flags);
>> You have to change ndo_bridge_setlink in netdevice.h first.
>> Otherwise when only this patch is applied (during bisection)
>> this won't compile.
>
>ack, will fix it and keep that in mind next time.
>>
>>>+ }
>>>+
>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>> I do not understand why to iterate over lower devices. At this
>> stage we don't know a thing about this upper or its lowers. Let
>> the uppers (/masters) to decide if this needs to be propagated
>> or not.
>
>Jiri, In the stacked devices case, there is no way to propagate the bridge
>port attributes to switch device driver today (vlan and other bridge port
>attributes). Can you tell me if there is a way ?. no, ndo_vlan* ndo's are not
>useful here. Nor we should go and implement ndo_bridge_setlink* in all
>devices that can be bridge ports.
Hmm. I just think that is cleaner to implement ndo_bridge_setlink in
bonding for example and let it propagate the the call to slaves.
Let every "upper" to handle ndo_bridge_setlink their way. Sometimes it
might not make sense to propagate to "lowers".
>
>And this allows a switch driver to receive these callbacks if it has marked
>the switch port with an offload flag. Your way of using the switch port to
>get to the switch driver does not help in these cases.
I do not follow how this is related to this case (stacked layout).
>
>The other option is to use the 'switch device (not port)' to get to the
>switch driver.
That would not help this case (stacked layout) I believe.
>This patch shows that you can still do this with the ndo ops.
>>
>>>+ err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>>>+ if (err)
>>>+ ret = err;
>>>+ }
>> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>>
>>>+
>>>+ return ret;
>>>+}
>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>>>+
>>>+/**
>>>+ * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>>>+ * attribute delete
>>>+ *
>>>+ * @dev: port device
>>>+ * @nlh: netlink msg with bridge port attributes
>>>+ *
>>>+ * Notify switch device port of bridge port attribute delete
>>>+ */
>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags)
>>>+{
>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>+ struct net_device *lower_dev;
>>>+ struct list_head *iter;
>>>+ int ret = 0, err = 0;
>>>+
>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>+ return err;
>>>+
>>>+ if (ops->ndo_bridge_dellink) {
>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>+ return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>+ }
>>>+
>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>+ err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>>>+ if (err)
>>>+ ret = err;
>>>+ }
>>>+
>>>+ return ret;
>>>+}
>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>--
>>>1.7.10.4
>>>
>
^ permalink raw reply
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: Murali Karicheri @ 2014-12-11 17:17 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, WingMan Kwok
In-Reply-To: <20141211.120104.312460507509497826.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On 12/11/2014 12:01 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2-l0cyMroinI0@public.gmane.org>
> Date: Thu, 11 Dec 2014 09:14:37 -0500
>
>> BTW, could you provide any suggestions that would help us merge this
>> series to upstream? This has been sitting on this list for a while
>> now.
>
> You simply have to continue going through the review and revision
> process until people no longer find problems with your changes.
>
> This could take several more weeks, you simply must be patient.
Ok. Thanks. That is encouraging to hear!
Regards,
--
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Roopa Prabhu @ 2014-12-11 17:32 UTC (permalink / raw)
To: Hubert Sokolowski; +Cc: netdev, Jamal Hadi Salim
In-Reply-To: <7968540cd0768a770b0c8b29ce41a162.squirrel@poczta.wsisiz.edu.pl>
On 12/11/14, 9:06 AM, Hubert Sokolowski wrote:
> My apologies for sending again, I forgot to include a sample output you asked.
>
>> Also, if i hear your concern correctly, for bridge ports that implement
>> ndo_fdb_dump, with commit 5e6d243587990a588143b9da3974833649595587, we
>> will get two entries for each 'self' entry above.
>> Can you also paste sample output for that ?.
>>
> [root@silpixa00378825 ~]# bridge fdb show brport mac0
> 33:33:00:00:00:01 self permanent
> 33:33:00:00:00:01 self permanent
>
>
Thanks. yes, that is a problem. And, this mac0 is not a bridge port
correct ?.
But, for the same test case, when mac0 is a bridge port, does your patch
under review make both the entries go away for a bridge port ?.
(If i understand jamal correctly, this is his concern).
^ permalink raw reply
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: Marcelo Ricardo Leitner @ 2014-12-11 17:33 UTC (permalink / raw)
To: vadim4j, Jiri Benc; +Cc: netdev
In-Reply-To: <20141211163335.GA27913@angus-think.wlc.globallogic.com>
On 11-12-2014 14:33, vadim4j@gmail.com wrote:
> On Thu, Dec 11, 2014 at 05:09:28PM +0100, Jiri Benc wrote:
>> On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote:
>>> From: Vadim Kochan <vadim4j@gmail.com>
>>>
>>> Added new '-ns' option to simplify executing following cmd:
>>>
>>> ip netns exec NETNS ip OPTIONS COMMAND OBJECT
>>>
>>> to
>>>
>>> ip -ns NETNS OPTIONS COMMAND OBJECT
>>>
>>> e.g.:
>>>
>>> ip -ns vnet0 link add br0 type bridge
>>
>> This is great! It's a thing that has been bothering me for long time
>> but never got high enough on my todo list. Thanks for working on this.
>>
>> However,
>>
>>> --- a/ip/ip.c
>>> +++ b/ip/ip.c
>>> @@ -262,6 +262,12 @@ int main(int argc, char **argv)
>>> rcvbuf = size;
>>> } else if (matches(opt, "-help") == 0) {
>>> usage();
>>> + } else if (matches(opt, "-ns") == 0) {
>>> + argc--;
>>> + argv++;
>>> + argv[0] = argv[1];
>>> + argv[1] = basename;
>>> + return netns_exec(argc, argv);
>>
>> I very much dislike this. There's no reason to exec another ip binary.
>> The main reason I wanted the -n (or whatever) option was to speed up
>> execution of test scripts in environments with hundreds of interfaces
>> in different net namespaces.
>>
>> Please just change to the specified netns and continue with interpreting
>> of the rest of the command line, there's absolutely no reason for doing
>> the exec.
> Yes, I will follow that way.
In that case, it would be interesting to also accelerate the original use
case, no? So all usages we currently have will benefit from this speed up
without a change.
if (command to be executed == myself)
switch namespace, continue without fork/exec..
I'm not sure if this is feasible, though. Just sharing the idea, didn't even
open the code..
Marcelo
^ permalink raw reply
* Re: [PATCH net v2] gianfar: Fix dma check map error when DMA_API_DEBUG is enabled
From: Claudiu Manoil @ 2014-12-11 17:05 UTC (permalink / raw)
To: Kevin Hao, netdev; +Cc: David Miller
In-Reply-To: <1418278121-20209-1-git-send-email-haokexin@gmail.com>
On 12/11/2014 8:08 AM, Kevin Hao wrote:
> We need to use dma_mapping_error() to check the dma address returned
> by dma_map_single/page(). Otherwise we would get warning like this:
> WARNING: at lib/dma-debug.c:1140
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.18.0-rc2-next-20141029 #196
> task: c0834300 ti: effe6000 task.ti: c0874000
> NIP: c02b2c98 LR: c02b2c98 CTR: c030abc4
> REGS: effe7d70 TRAP: 0700 Not tainted (3.18.0-rc2-next-20141029)
> MSR: 00021000 <CE,ME> CR: 22044022 XER: 20000000
>
> GPR00: c02b2c98 effe7e20 c0834300 00000098 00021000 00000000 c030b898 00000003
> GPR08: 00000001 00000000 00000001 749eec9d 22044022 1001abe0 00000020 ef278678
> GPR16: ef278670 ef278668 ef278660 070a8040 c087f99c c08cdc60 00029000 c0840d44
> GPR24: c08be6e8 c0840000 effe7e78 ef041340 00000600 ef114e10 00000000 c08be6e0
> NIP [c02b2c98] check_unmap+0x51c/0x9e4
> LR [c02b2c98] check_unmap+0x51c/0x9e4
> Call Trace:
> [effe7e20] [c02b2c98] check_unmap+0x51c/0x9e4 (unreliable)
> [effe7e70] [c02b31d8] debug_dma_unmap_page+0x78/0x8c
> [effe7ed0] [c03d1640] gfar_clean_rx_ring+0x208/0x488
> [effe7f40] [c03d1a9c] gfar_poll_rx_sq+0x3c/0xa8
> [effe7f60] [c04f8714] net_rx_action+0xc0/0x178
> [effe7f90] [c00435a0] __do_softirq+0x100/0x1fc
> [effe7fe0] [c0043958] irq_exit+0xa4/0xc8
> [effe7ff0] [c000d14c] call_do_irq+0x24/0x3c
> [c0875e90] [c00048a0] do_IRQ+0x8c/0xf8
> [c0875eb0] [c000ed10] ret_from_except+0x0/0x18
>
> For TX, we need to unmap the pages which has already been mapped and
> free the skb before return.
>
> For RX, move the dma mapping and error check to gfar_new_skb(). We
> would reuse the original skb in the rx ring when either allocating
> skb failure or dma mapping error.
>
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
> ---
> v2: Just update the RX path to reuse the original skb when dma mapping error
> occurs as suggested by David.
>
Very nice refactoring of gfar_new_skb(), removing gfar_new_rxbdp() in
the process. (Did not have the time to test it yet.)
Thanks.
Reviewed-by: Claudiu Manoil <claudiu.manoil@freescale.com>
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Roopa Prabhu @ 2014-12-11 17:41 UTC (permalink / raw)
To: Jiri Pirko
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141211165627.GF1912@nanopsycho.orion>
On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>> -----Original Message-----
>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>> To: Jiri Pirko
>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>> kernel@vger.kernel.org
>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>> configuration
>>>>>
>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>>>> From: Marco Varlese <marco.varlese@intel.com>
>>>>>>>
>>>>>>> Switch hardware offers a list of attributes that are configurable on
>>>>>>> a per port basis.
>>>>>>> This patch provides a mechanism to configure switch ports by adding
>>>>>>> an NDO for setting specific values to specific attributes.
>>>>>>> There will be a separate patch that extends iproute2 to call the new
>>>>>>> NDO.
>>>>>> What are these attributes? Can you give some examples. I'm asking
>>>>>> because there is a plan to pass generic attributes to switch ports
>>>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>> bridge is setting that attribute.
>>>>>>
>>>>>> Is there need to set something directly from userspace or does it make
>>>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>> be needed.
>>>>> +1
>>>>>
>>>>> I think for many attributes it would be best to have both. The in kernel callers
>>>>> and netlink userspace can use the same driver ndo_ops.
>>>>>
>>>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>> may have some attributes that are not specific to any existing software
>>>>> module. I'm guessing Marco has some examples of these.
>>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>> --
>>>>> John Fastabend Intel Corporation
>>>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>
>>>> An example of attributes are:
>>>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>
>>>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>
>>>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>
>>>> I hope this clarifies some points.
>>> It does. Makes sense. We need to expose this attr set/get for both
>>> in-kernel and userspace use cases.
>>>
>>> Please adjust you patch for this. Also, as a second patch, it would be
>>> great if you can convert ndo_switch_port_stp_update to this new ndo.
>> Why are we exposing generic switch attribute get/set from userspace ?. We
>> already have specific attributes for learning/flooding which can be extended
>> further.
> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
> might be another generic attrs, no?
I cant think of any. And plus, the whole point of switchdev l2 offloads
was to map these to existing
bridge attributes. And we already have a match for some of the
attributes that marco wants.
If there is a need for such attributes, i don't see why it is needed for
switch devices only.
It is needed for any hw (nics etc). And, a precedence to this is to do
it via ethtool.
Having said that, am sure we will find a need for this in the future.
And having a netlink attribute always helps.
Today, it seems like these can be mapped to existing attributes that are
settable via ndo_bridge_setlink/getlink.
>
>> And for in kernel api....i had a sample patch in my RFC series (Which i was
>> going to resubmit, until it was decided that we will use existing api around
>> ndo_bridge_setlink/ndo_bridge_getlink):
>> http://www.spinics.net/lists/netdev/msg305473.html
> Yes, this might become handy for other generic non-bridge attrs.
>
>> Thanks,
>> Roopa
>>
>>
>>
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-11 17:54 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <5489D739.9010303@cumulusnetworks.com>
Thu, Dec 11, 2014 at 06:41:13PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>>>On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>>Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>>-----Original Message-----
>>>>>>From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>>Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>>To: Jiri Pirko
>>>>>>Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>>stephen@networkplumber.org; Fastabend, John R;
>>>>>>roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>>kernel@vger.kernel.org
>>>>>>Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>>>configuration
>>>>>>
>>>>>>On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>>Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>>>>>From: Marco Varlese <marco.varlese@intel.com>
>>>>>>>>
>>>>>>>>Switch hardware offers a list of attributes that are configurable on
>>>>>>>>a per port basis.
>>>>>>>>This patch provides a mechanism to configure switch ports by adding
>>>>>>>>an NDO for setting specific values to specific attributes.
>>>>>>>>There will be a separate patch that extends iproute2 to call the new
>>>>>>>>NDO.
>>>>>>>What are these attributes? Can you give some examples. I'm asking
>>>>>>>because there is a plan to pass generic attributes to switch ports
>>>>>>>replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>>>bridge is setting that attribute.
>>>>>>>
>>>>>>>Is there need to set something directly from userspace or does it make
>>>>>>>rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>>>be needed.
>>>>>>+1
>>>>>>
>>>>>>I think for many attributes it would be best to have both. The in kernel callers
>>>>>>and netlink userspace can use the same driver ndo_ops.
>>>>>>
>>>>>>But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>>>may have some attributes that are not specific to any existing software
>>>>>>module. I'm guessing Marco has some examples of these.
>>>>>>
>>>>>>[...]
>>>>>>
>>>>>>
>>>>>>--
>>>>>>John Fastabend Intel Corporation
>>>>>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>>
>>>>>An example of attributes are:
>>>>>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>>>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>>>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>>
>>>>>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>>
>>>>>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>>
>>>>>I hope this clarifies some points.
>>>>It does. Makes sense. We need to expose this attr set/get for both
>>>>in-kernel and userspace use cases.
>>>>
>>>>Please adjust you patch for this. Also, as a second patch, it would be
>>>>great if you can convert ndo_switch_port_stp_update to this new ndo.
>>>Why are we exposing generic switch attribute get/set from userspace ?. We
>>>already have specific attributes for learning/flooding which can be extended
>>>further.
>>Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>>might be another generic attrs, no?
>I cant think of any. And plus, the whole point of switchdev l2 offloads was
>to map these to existing
>bridge attributes. And we already have a match for some of the attributes
>that marco wants.
>
>If there is a need for such attributes, i don't see why it is needed for
>switch devices only.
>It is needed for any hw (nics etc). And, a precedence to this is to do it via
>ethtool.
Fair enough.
>
>Having said that, am sure we will find a need for this in the future. And
>having a netlink attribute always helps.
>
>Today, it seems like these can be mapped to existing attributes that are
>settable via ndo_bridge_setlink/getlink.
Okay. That makes sense so far for bridge.
>
>>
>>>And for in kernel api....i had a sample patch in my RFC series (Which i was
>>>going to resubmit, until it was decided that we will use existing api around
>>>ndo_bridge_setlink/ndo_bridge_getlink):
>>>http://www.spinics.net/lists/netdev/msg305473.html
>>Yes, this might become handy for other generic non-bridge attrs.
>>
>>>Thanks,
>>>Roopa
>>>
>>>
>>>
>
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: John Fastabend @ 2014-12-11 17:55 UTC (permalink / raw)
To: Roopa Prabhu, Jiri Pirko
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <5489D739.9010303@cumulusnetworks.com>
On 12/11/2014 09:41 AM, Roopa Prabhu wrote:
> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>> -----Original Message-----
>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>> To: Jiri Pirko
>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>> kernel@vger.kernel.org
>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>>> configuration
>>>>>>
>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>>>>> From: Marco Varlese <marco.varlese@intel.com>
>>>>>>>>
>>>>>>>> Switch hardware offers a list of attributes that are configurable on
>>>>>>>> a per port basis.
>>>>>>>> This patch provides a mechanism to configure switch ports by adding
>>>>>>>> an NDO for setting specific values to specific attributes.
>>>>>>>> There will be a separate patch that extends iproute2 to call the new
>>>>>>>> NDO.
>>>>>>> What are these attributes? Can you give some examples. I'm asking
>>>>>>> because there is a plan to pass generic attributes to switch ports
>>>>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>>> bridge is setting that attribute.
>>>>>>>
>>>>>>> Is there need to set something directly from userspace or does it make
>>>>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>>> be needed.
>>>>>> +1
>>>>>>
>>>>>> I think for many attributes it would be best to have both. The in kernel callers
>>>>>> and netlink userspace can use the same driver ndo_ops.
>>>>>>
>>>>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>>> may have some attributes that are not specific to any existing software
>>>>>> module. I'm guessing Marco has some examples of these.
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>> --
>>>>>> John Fastabend Intel Corporation
>>>>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>>
>>>>> An example of attributes are:
>>>>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>>
>>>>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>>
>>>>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>>
>>>>> I hope this clarifies some points.
>>>> It does. Makes sense. We need to expose this attr set/get for both
>>>> in-kernel and userspace use cases.
>>>>
>>>> Please adjust you patch for this. Also, as a second patch, it would be
>>>> great if you can convert ndo_switch_port_stp_update to this new ndo.
>>> Why are we exposing generic switch attribute get/set from userspace ?. We
>>> already have specific attributes for learning/flooding which can be extended
>>> further.
>> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>> might be another generic attrs, no?
> I cant think of any. And plus, the whole point of switchdev l2 offloads was to map these to existing
> bridge attributes. And we already have a match for some of the attributes that marco wants.
>
> If there is a need for such attributes, i don't see why it is needed for switch devices only.
> It is needed for any hw (nics etc). And, a precedence to this is to do it via ethtool.
I would prefer to _not_ add more attributes to ethtool. 'ethtool' is in general
harder to work with then netlink for all but the most static attributes.
>
> Having said that, am sure we will find a need for this in the future. And having a netlink attribute always helps.
>
> Today, it seems like these can be mapped to existing attributes that are settable via ndo_bridge_setlink/getlink.
Absolutely I view this as an RFC patch noting we may/will need some extensions
in the future. .We can evaluate the attributes on a case by case basis as they
come in. And if they all fit in setlink/getlink that is great.
>
>>
>>> And for in kernel api....i had a sample patch in my RFC series (Which i was
>>> going to resubmit, until it was decided that we will use existing api around
>>> ndo_bridge_setlink/ndo_bridge_getlink):
>>> http://www.spinics.net/lists/netdev/msg305473.html
>> Yes, this might become handy for other generic non-bridge attrs.
>>
>>> Thanks,
>>> Roopa
>>>
>>>
>>>
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Roopa Prabhu @ 2014-12-11 17:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <20141211171145.GG1912@nanopsycho.orion>
On 12/11/14, 9:11 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 05:52:10PM CET, roopa@cumulusnetworks.com wrote:
>> On 12/10/14, 1:37 AM, Jiri Pirko wrote:
>>> Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This patch adds two new api's netdev_switch_port_bridge_setlink
>>>> and netdev_switch_port_bridge_dellink to offload bridge port attributes
>>>> to switch asic
>>>>
>>>> (The names of the apis look odd with 'switch_port_bridge',
>>>> but am more inclined to change the prefix of the api to something else.
>>>> Will take any suggestions).
>>>>
>>>> The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>>>> pass bridge port attributes to the port device.
>>>>
>>>> If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>>>> the bridge port attribute offload ndo, call bridge port attribute ndo's on
>>>> the lowerdevs if supported. This is one way to pass bridge port attributes
>>>> through stacked netdevs (example when bridge port is a bond and bond slaves
>>>> are switch ports).
>>>>
>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>> ---
>>>> include/net/switchdev.h | 5 +++-
>>>> net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 74 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>> index 8a6d164..22676b6 100644
>>>> --- a/include/net/switchdev.h
>>>> +++ b/include/net/switchdev.h
>>>> @@ -17,7 +17,10 @@
>>>> int netdev_switch_parent_id_get(struct net_device *dev,
>>>> struct netdev_phys_item_id *psid);
>>>> int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>> -
>>>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>> + struct nlmsghdr *nlh, u16 flags);
>>>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>> + struct nlmsghdr *nlh, u16 flags);
>>>> #else
>>>>
>>>> static inline int netdev_switch_parent_id_get(struct net_device *dev,
>>>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>> index d162b21..62317e1 100644
>>>> --- a/net/switchdev/switchdev.c
>>>> +++ b/net/switchdev/switchdev.c
>>>> @@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>>> return ops->ndo_switch_port_stp_update(dev, state);
>>>> }
>>>> EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>> +
>>>> +/**
>>>> + * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>>>> + * port attributes
>>>> + *
>>>> + * @dev: port device
>>>> + * @nlh: netlink msg with bridge port attributes
>>>> + *
>>>> + * Notify switch device port of bridge port attributes
>>>> + */
>>>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>> + struct nlmsghdr *nlh, u16 flags)
>>>> +{
>>>> + const struct net_device_ops *ops = dev->netdev_ops;
>>>> + struct net_device *lower_dev;
>>>> + struct list_head *iter;
>>>> + int ret = 0, err = 0;
>>>> +
>>>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>> + return err;
>>>> +
>>>> + if (ops->ndo_bridge_setlink) {
>>>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>>>> + return ops->ndo_bridge_setlink(dev, nlh, flags);
>>> You have to change ndo_bridge_setlink in netdevice.h first.
>>> Otherwise when only this patch is applied (during bisection)
>>> this won't compile.
>> ack, will fix it and keep that in mind next time.
>>>> + }
>>>> +
>>>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>> I do not understand why to iterate over lower devices. At this
>>> stage we don't know a thing about this upper or its lowers. Let
>>> the uppers (/masters) to decide if this needs to be propagated
>>> or not.
>> Jiri, In the stacked devices case, there is no way to propagate the bridge
>> port attributes to switch device driver today (vlan and other bridge port
>> attributes). Can you tell me if there is a way ?. no, ndo_vlan* ndo's are not
>> useful here. Nor we should go and implement ndo_bridge_setlink* in all
>> devices that can be bridge ports.
>
> Hmm. I just think that is cleaner to implement ndo_bridge_setlink in
> bonding for example and let it propagate the the call to slaves.
No, that will require bridge attribute support in all drivers. And that
is no good.
> Let every "upper" to handle ndo_bridge_setlink their way. Sometimes it
> might not make sense to propagate to "lowers".
This does not really propagate to lowers. It is just trying to get to a
switch port and from there to the switch driver.
Example, bond driver does not need to care if its a bridge port. It will
simply pass the call to its slave which
might be a switch port.
bond driver does not care if its a bridge port. But the switch driver
cares, because it knows that the bond was created with switch ports.
>
>> And this allows a switch driver to receive these callbacks if it has marked
>> the switch port with an offload flag. Your way of using the switch port to
>> get to the switch driver does not help in these cases.
> I do not follow how this is related to this case (stacked layout).
>
>> The other option is to use the 'switch device (not port)' to get to the
>> switch driver.
>
> That would not help this case (stacked layout) I believe.
>
>
>> This patch shows that you can still do this with the ndo ops.
>>>> + err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>>>> + if (err)
>>>> + ret = err;
>>>> + }
>>> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>>>
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>>>> +
>>>> +/**
>>>> + * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>>>> + * attribute delete
>>>> + *
>>>> + * @dev: port device
>>>> + * @nlh: netlink msg with bridge port attributes
>>>> + *
>>>> + * Notify switch device port of bridge port attribute delete
>>>> + */
>>>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>> + struct nlmsghdr *nlh, u16 flags)
>>>> +{
>>>> + const struct net_device_ops *ops = dev->netdev_ops;
>>>> + struct net_device *lower_dev;
>>>> + struct list_head *iter;
>>>> + int ret = 0, err = 0;
>>>> +
>>>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>> + return err;
>>>> +
>>>> + if (ops->ndo_bridge_dellink) {
>>>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>>>> + return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>> + }
>>>> +
>>>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>> + err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>>>> + if (err)
>>>> + ret = err;
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>> --
>>>> 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
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Jiri Pirko @ 2014-12-11 18:07 UTC (permalink / raw)
To: Roopa Prabhu
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <5489DB73.1080808@cumulusnetworks.com>
Thu, Dec 11, 2014 at 06:59:15PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 9:11 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 05:52:10PM CET, roopa@cumulusnetworks.com wrote:
>>>On 12/10/14, 1:37 AM, Jiri Pirko wrote:
>>>>Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>>>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>
>>>>>This patch adds two new api's netdev_switch_port_bridge_setlink
>>>>>and netdev_switch_port_bridge_dellink to offload bridge port attributes
>>>>>to switch asic
>>>>>
>>>>>(The names of the apis look odd with 'switch_port_bridge',
>>>>>but am more inclined to change the prefix of the api to something else.
>>>>>Will take any suggestions).
>>>>>
>>>>>The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>>>>>pass bridge port attributes to the port device.
>>>>>
>>>>>If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>>>>>the bridge port attribute offload ndo, call bridge port attribute ndo's on
>>>>>the lowerdevs if supported. This is one way to pass bridge port attributes
>>>>>through stacked netdevs (example when bridge port is a bond and bond slaves
>>>>>are switch ports).
>>>>>
>>>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>---
>>>>>include/net/switchdev.h | 5 +++-
>>>>>net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>2 files changed, 74 insertions(+), 1 deletion(-)
>>>>>
>>>>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>>>index 8a6d164..22676b6 100644
>>>>>--- a/include/net/switchdev.h
>>>>>+++ b/include/net/switchdev.h
>>>>>@@ -17,7 +17,10 @@
>>>>>int netdev_switch_parent_id_get(struct net_device *dev,
>>>>> struct netdev_phys_item_id *psid);
>>>>>int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>>>-
>>>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>>>+ struct nlmsghdr *nlh, u16 flags);
>>>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>+ struct nlmsghdr *nlh, u16 flags);
>>>>>#else
>>>>>
>>>>>static inline int netdev_switch_parent_id_get(struct net_device *dev,
>>>>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>>>index d162b21..62317e1 100644
>>>>>--- a/net/switchdev/switchdev.c
>>>>>+++ b/net/switchdev/switchdev.c
>>>>>@@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>>>> return ops->ndo_switch_port_stp_update(dev, state);
>>>>>}
>>>>>EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>>>+
>>>>>+/**
>>>>>+ * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>>>>>+ * port attributes
>>>>>+ *
>>>>>+ * @dev: port device
>>>>>+ * @nlh: netlink msg with bridge port attributes
>>>>>+ *
>>>>>+ * Notify switch device port of bridge port attributes
>>>>>+ */
>>>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>>>+ struct nlmsghdr *nlh, u16 flags)
>>>>>+{
>>>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>>>+ struct net_device *lower_dev;
>>>>>+ struct list_head *iter;
>>>>>+ int ret = 0, err = 0;
>>>>>+
>>>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>+ return err;
>>>>>+
>>>>>+ if (ops->ndo_bridge_setlink) {
>>>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>+ return ops->ndo_bridge_setlink(dev, nlh, flags);
>>>> You have to change ndo_bridge_setlink in netdevice.h first.
>>>> Otherwise when only this patch is applied (during bisection)
>>>> this won't compile.
>>>ack, will fix it and keep that in mind next time.
>>>>>+ }
>>>>>+
>>>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>> I do not understand why to iterate over lower devices. At this
>>>> stage we don't know a thing about this upper or its lowers. Let
>>>> the uppers (/masters) to decide if this needs to be propagated
>>>> or not.
>>>Jiri, In the stacked devices case, there is no way to propagate the bridge
>>>port attributes to switch device driver today (vlan and other bridge port
>>>attributes). Can you tell me if there is a way ?. no, ndo_vlan* ndo's are not
>>>useful here. Nor we should go and implement ndo_bridge_setlink* in all
>>>devices that can be bridge ports.
>>
>>Hmm. I just think that is cleaner to implement ndo_bridge_setlink in
>>bonding for example and let it propagate the the call to slaves.
>No, that will require bridge attribute support in all drivers. And that is no
>good.
Not all drivers, just all masters which want to support this. Like bond,
team, macvlan etc. That would be the same as for
ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid/ndo_change_mtu etc. I do not
see any problem in that. It is much much clearer over big hammer iterate
over lowers in my opinion.
>>Let every "upper" to handle ndo_bridge_setlink their way. Sometimes it
>>might not make sense to propagate to "lowers".
>
>This does not really propagate to lowers. It is just trying to get to a
>switch port and from there to the switch driver.
>Example, bond driver does not need to care if its a bridge port. It will
>simply pass the call to its slave which
>might be a switch port.
>
>bond driver does not care if its a bridge port. But the switch driver cares,
>because it knows that the bond was created with switch ports.
>
>
>>
>>>And this allows a switch driver to receive these callbacks if it has marked
>>>the switch port with an offload flag. Your way of using the switch port to
>>>get to the switch driver does not help in these cases.
>>I do not follow how this is related to this case (stacked layout).
>>
>>>The other option is to use the 'switch device (not port)' to get to the
>>>switch driver.
>>
>>That would not help this case (stacked layout) I believe.
>>
>>
>>>This patch shows that you can still do this with the ndo ops.
>>>>>+ err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>>>>>+ if (err)
>>>>>+ ret = err;
>>>>>+ }
>>>> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>>>>
>>>>>+
>>>>>+ return ret;
>>>>>+}
>>>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>>>>>+
>>>>>+/**
>>>>>+ * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>>>>>+ * attribute delete
>>>>>+ *
>>>>>+ * @dev: port device
>>>>>+ * @nlh: netlink msg with bridge port attributes
>>>>>+ *
>>>>>+ * Notify switch device port of bridge port attribute delete
>>>>>+ */
>>>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>+ struct nlmsghdr *nlh, u16 flags)
>>>>>+{
>>>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>>>+ struct net_device *lower_dev;
>>>>>+ struct list_head *iter;
>>>>>+ int ret = 0, err = 0;
>>>>>+
>>>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>+ return err;
>>>>>+
>>>>>+ if (ops->ndo_bridge_dellink) {
>>>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>+ return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>>>+ }
>>>>>+
>>>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>>>+ err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>>>>>+ if (err)
>>>>>+ ret = err;
>>>>>+ }
>>>>>+
>>>>>+ return ret;
>>>>>+}
>>>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>>>--
>>>>>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
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: Jiri Benc @ 2014-12-11 18:08 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: vadim4j, netdev
In-Reply-To: <5489D56E.4010505@gmail.com>
On Thu, 11 Dec 2014 15:33:34 -0200, Marcelo Ricardo Leitner wrote:
> In that case, it would be interesting to also accelerate the original use
> case, no? So all usages we currently have will benefit from this speed up
> without a change.
>
> if (command to be executed == myself)
> switch namespace, continue without fork/exec..
It's never good idea to do such tricks behind the user's back. This
particular case could easily break for users wanting to execute a
different ip binary (for whatever reason).
All programs should do what they are told to do, not try to outsmart
the user.
Jiri
--
Jiri Benc
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Roopa Prabhu @ 2014-12-11 18:27 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <20141211180743.GA2010@nanopsycho.orion>
On 12/11/14, 10:07 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 06:59:15PM CET, roopa@cumulusnetworks.com wrote:
>> On 12/11/14, 9:11 AM, Jiri Pirko wrote:
>>> Thu, Dec 11, 2014 at 05:52:10PM CET, roopa@cumulusnetworks.com wrote:
>>>> On 12/10/14, 1:37 AM, Jiri Pirko wrote:
>>>>> Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>>>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>
>>>>>> This patch adds two new api's netdev_switch_port_bridge_setlink
>>>>>> and netdev_switch_port_bridge_dellink to offload bridge port attributes
>>>>>> to switch asic
>>>>>>
>>>>>> (The names of the apis look odd with 'switch_port_bridge',
>>>>>> but am more inclined to change the prefix of the api to something else.
>>>>>> Will take any suggestions).
>>>>>>
>>>>>> The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>>>>>> pass bridge port attributes to the port device.
>>>>>>
>>>>>> If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>>>>>> the bridge port attribute offload ndo, call bridge port attribute ndo's on
>>>>>> the lowerdevs if supported. This is one way to pass bridge port attributes
>>>>>> through stacked netdevs (example when bridge port is a bond and bond slaves
>>>>>> are switch ports).
>>>>>>
>>>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>> ---
>>>>>> include/net/switchdev.h | 5 +++-
>>>>>> net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 74 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>>>> index 8a6d164..22676b6 100644
>>>>>> --- a/include/net/switchdev.h
>>>>>> +++ b/include/net/switchdev.h
>>>>>> @@ -17,7 +17,10 @@
>>>>>> int netdev_switch_parent_id_get(struct net_device *dev,
>>>>>> struct netdev_phys_item_id *psid);
>>>>>> int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>>>> -
>>>>>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>>>> + struct nlmsghdr *nlh, u16 flags);
>>>>>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>> + struct nlmsghdr *nlh, u16 flags);
>>>>>> #else
>>>>>>
>>>>>> static inline int netdev_switch_parent_id_get(struct net_device *dev,
>>>>>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>>>> index d162b21..62317e1 100644
>>>>>> --- a/net/switchdev/switchdev.c
>>>>>> +++ b/net/switchdev/switchdev.c
>>>>>> @@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>>>>> return ops->ndo_switch_port_stp_update(dev, state);
>>>>>> }
>>>>>> EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>>>> +
>>>>>> +/**
>>>>>> + * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>>>>>> + * port attributes
>>>>>> + *
>>>>>> + * @dev: port device
>>>>>> + * @nlh: netlink msg with bridge port attributes
>>>>>> + *
>>>>>> + * Notify switch device port of bridge port attributes
>>>>>> + */
>>>>>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>>>> + struct nlmsghdr *nlh, u16 flags)
>>>>>> +{
>>>>>> + const struct net_device_ops *ops = dev->netdev_ops;
>>>>>> + struct net_device *lower_dev;
>>>>>> + struct list_head *iter;
>>>>>> + int ret = 0, err = 0;
>>>>>> +
>>>>>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>> + return err;
>>>>>> +
>>>>>> + if (ops->ndo_bridge_setlink) {
>>>>>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>> + return ops->ndo_bridge_setlink(dev, nlh, flags);
>>>>> You have to change ndo_bridge_setlink in netdevice.h first.
>>>>> Otherwise when only this patch is applied (during bisection)
>>>>> this won't compile.
>>>> ack, will fix it and keep that in mind next time.
>>>>>> + }
>>>>>> +
>>>>>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>>> I do not understand why to iterate over lower devices. At this
>>>>> stage we don't know a thing about this upper or its lowers. Let
>>>>> the uppers (/masters) to decide if this needs to be propagated
>>>>> or not.
>>>> Jiri, In the stacked devices case, there is no way to propagate the bridge
>>>> port attributes to switch device driver today (vlan and other bridge port
>>>> attributes). Can you tell me if there is a way ?. no, ndo_vlan* ndo's are not
>>>> useful here. Nor we should go and implement ndo_bridge_setlink* in all
>>>> devices that can be bridge ports.
>>> Hmm. I just think that is cleaner to implement ndo_bridge_setlink in
>>> bonding for example and let it propagate the the call to slaves.
>> No, that will require bridge attribute support in all drivers. And that is no
>> good.
> Not all drivers, just all masters which want to support this. Like bond,
> team, macvlan etc. That would be the same as for
> ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid/ndo_change_mtu etc. I do not
> see any problem in that. It is much much clearer over big hammer iterate
> over lowers in my opinion.
You cannot avoid the lowerdev iteration in any case.
If you added it in the individual drivers: bond, macvlan and other
drivers will all have to do the same thing.
ie Call bridge setlink on lowerdevs.
My patch avoids the need to modify these drivers. Besides it does this
only when the OFFLOAD flag is set.
It will not stop at adding the ndo_bridge_setlink to bond/macvlan etc.
It will be all other ndo_ops we will need for switch asics.
It will be l3 tomorrow, if the route is through a bond (But at that
point, we may end up having to introduce switch device instead of going
to the port. Lets see).
Today this patch introduces an abstract way to get to the switch driver
by getting to the slave switch port (And only when the OFFLOAD flag is set).
>
>
>>> Let every "upper" to handle ndo_bridge_setlink their way. Sometimes it
>>> might not make sense to propagate to "lowers".
>> This does not really propagate to lowers. It is just trying to get to a
>> switch port and from there to the switch driver.
>> Example, bond driver does not need to care if its a bridge port. It will
>> simply pass the call to its slave which
>> might be a switch port.
>>
>> bond driver does not care if its a bridge port. But the switch driver cares,
>> because it knows that the bond was created with switch ports.
>>
>>
>>>> And this allows a switch driver to receive these callbacks if it has marked
>>>> the switch port with an offload flag. Your way of using the switch port to
>>>> get to the switch driver does not help in these cases.
>>> I do not follow how this is related to this case (stacked layout).
>>>
>>>> The other option is to use the 'switch device (not port)' to get to the
>>>> switch driver.
>>> That would not help this case (stacked layout) I believe.
>>>
>>>
>>>> This patch shows that you can still do this with the ndo ops.
>>>>>> + err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>>>>>> + if (err)
>>>>>> + ret = err;
>>>>>> + }
>>>>> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>>>>>
>>>>>> +
>>>>>> + return ret;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>>>>>> +
>>>>>> +/**
>>>>>> + * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>>>>>> + * attribute delete
>>>>>> + *
>>>>>> + * @dev: port device
>>>>>> + * @nlh: netlink msg with bridge port attributes
>>>>>> + *
>>>>>> + * Notify switch device port of bridge port attribute delete
>>>>>> + */
>>>>>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>> + struct nlmsghdr *nlh, u16 flags)
>>>>>> +{
>>>>>> + const struct net_device_ops *ops = dev->netdev_ops;
>>>>>> + struct net_device *lower_dev;
>>>>>> + struct list_head *iter;
>>>>>> + int ret = 0, err = 0;
>>>>>> +
>>>>>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>> + return err;
>>>>>> +
>>>>>> + if (ops->ndo_bridge_dellink) {
>>>>>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>> + return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>>>> + }
>>>>>> +
>>>>>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>>>> + err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>>>>>> + if (err)
>>>>>> + ret = err;
>>>>>> + }
>>>>>> +
>>>>>> + return ret;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>>>> --
>>>>>> 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox