* 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
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: Joe Perches @ 2014-12-11 18:34 UTC (permalink / raw)
To: Murali Karicheri
Cc: David Miller, netdev, linux-arm-kernel, linux-kernel, devicetree,
robh+dt, grant.likely, WingMan Kwok
In-Reply-To: <5489D196.4070703@ti.com>
On Thu, 2014-12-11 at 12:17 -0500, Murali Karicheri wrote:
> On 12/11/2014 12:01 PM, David Miller wrote:
> > 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.
>
> Ok. Thanks. That is encouraging to hear!
Perhaps these trivial things might be considered
(uncompiled)
---
drivers/net/ethernet/ti/netcp_core.c | 2 +-
drivers/net/ethernet/ti/netcp_ethss.c | 83 +++++++++++++++++---------------
drivers/net/ethernet/ti/netcp_xgbepcsr.c | 48 +++++++++---------
3 files changed, 71 insertions(+), 62 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 60ad299..8f38fe8 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1094,7 +1094,7 @@ static void netcp_tx_notify(void *arg)
napi_schedule(&netcp->tx_napi);
}
-static struct knav_dma_desc*
+static struct knav_dma_desc *
netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
{
struct knav_dma_desc *desc, *ndesc, *pdesc;
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 036b886..3757957 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -486,21 +486,29 @@ struct netcp_ethtool_stat {
int offset;
};
-#define GBE_STATSA_INFO(field) "GBE_A:"#field, GBE_STATSA_MODULE,\
- FIELD_SIZEOF(struct gbe_hw_stats, field), \
- offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSB_INFO(field) "GBE_B:"#field, GBE_STATSB_MODULE,\
- FIELD_SIZEOF(struct gbe_hw_stats, field), \
- offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSC_INFO(field) "GBE_C:"#field, GBE_STATSC_MODULE,\
- FIELD_SIZEOF(struct gbe_hw_stats, field), \
- offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSD_INFO(field) "GBE_D:"#field, GBE_STATSD_MODULE,\
- FIELD_SIZEOF(struct gbe_hw_stats, field), \
- offsetof(struct gbe_hw_stats, field)
+#define GBE_STATSA_INFO(field) \
+ .desc = "GBE_A:"#field, \
+ .type = GBE_STATSA_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSB_INFO(field) \
+ .desc = "GBE_B:"#field, \
+ .type = GBE_STATSB_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSC_INFO(field) \
+ .desc = "GBE_C:"#field, \
+ .type = GBE_STATSC_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSD_INFO(field) \
+ .desc = "GBE_D:"#field, \
+ .type = GBE_STATSD_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
static const struct netcp_ethtool_stat gbe13_et_stats[] = {
/* GBE module A */
@@ -645,17 +653,23 @@ static const struct netcp_ethtool_stat gbe13_et_stats[] = {
{GBE_STATSD_INFO(rx_dma_overruns)},
};
-#define XGBE_STATS0_INFO(field) "GBE_0:"#field, XGBE_STATS0_MODULE, \
- FIELD_SIZEOF(struct xgbe_hw_stats, field), \
- offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS0_INFO(field) \
+ .desc = "GBE_0:"#field, \
+ .type = GBE_STATS0_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
-#define XGBE_STATS1_INFO(field) "GBE_1:"#field, XGBE_STATS1_MODULE, \
- FIELD_SIZEOF(struct xgbe_hw_stats, field), \
- offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS1_INFO(field) \
+ .desc = "GBE_1:"#field, \
+ .type = GBE_STATS1_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
-#define XGBE_STATS2_INFO(field) "GBE_2:"#field, XGBE_STATS2_MODULE, \
- FIELD_SIZEOF(struct xgbe_hw_stats, field), \
- offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS2_INFO(field)
+ .desc = "GBE_2:"#field, \
+ .type = GBE_STATS2_MODULE, \
+ .size = FIELD_SIZEOF(struct gbe_hw_stats, field), \
+ .offset = offsetof(struct gbe_hw_stats, field)
static const struct netcp_ethtool_stat xgbe10_et_stats[] = {
/* GBE module 0 */
@@ -883,11 +897,11 @@ static void gbe_update_stats_ver14(struct gbe_priv *gbe_dev, uint64_t *data)
case GBE_STATSA_MODULE:
case GBE_STATSC_MODULE:
base = gbe_statsa;
- break;
+ break;
case GBE_STATSB_MODULE:
case GBE_STATSD_MODULE:
base = gbe_statsb;
- break;
+ break;
}
p = base + gbe_dev->et_stats[j].offset;
@@ -1639,11 +1653,8 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
for_each_child_of_node(node, port) {
slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
- if (!slave) {
- dev_err(dev, "memomry alloc failed for secondary port(%s), skipping...\n",
- port->name);
+ if (!slave)
continue;
- }
if (init_slave(gbe_dev, slave, port)) {
dev_err(dev, "Failed to initialize secondary port(%s), skipping...\n",
@@ -1763,10 +1774,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
XGBE10_NUM_STAT_ENTRIES *
XGBE10_NUM_SLAVES * sizeof(u64),
GFP_KERNEL);
- if (!gbe_dev->hw_stats) {
- dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+ if (!gbe_dev->hw_stats)
return -ENOMEM;
- }
gbe_dev->ss_version = XGBE_SS_VERSION_10;
gbe_dev->sgmii_port_regs = gbe_dev->ss_regs +
@@ -1836,10 +1845,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
GBE13_NUM_HW_STAT_ENTRIES *
GBE13_NUM_SLAVES * sizeof(u64),
GFP_KERNEL);
- if (!gbe_dev->hw_stats) {
- dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+ if (!gbe_dev->hw_stats)
return -ENOMEM;
- }
gbe_dev->ss_version = GBE_SS_VERSION_14;
gbe_dev->sgmii_port_regs = regs + GBE13_SGMII_MODULE_OFFSET;
@@ -1995,10 +2002,10 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
dev_err(gbe_dev->dev, "error initializing ale engine\n");
ret = -ENODEV;
goto quit;
- } else {
- dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
}
+ dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
+
/* initialize host port */
gbe_init_host_port(gbe_dev);
diff --git a/drivers/net/ethernet/ti/netcp_xgbepcsr.c b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
index 33571ac..d93a6a4 100644
--- a/drivers/net/ethernet/ti/netcp_xgbepcsr.c
+++ b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
@@ -14,6 +14,9 @@
* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
+
+#define pr_fmt(fmt) "XGBE: " fmt
+
#include "netcp.h"
/* XGBE registers */
@@ -43,7 +46,7 @@ struct serdes_cfg {
u32 mask;
};
-static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
+static const struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
{0x0000, 0x00800002, 0x00ff00ff},
{0x0014, 0x00003838, 0x0000ffff},
{0x0060, 0x1c44e438, 0xffffffff},
@@ -54,7 +57,7 @@ static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
{0x0000, 0x00000003, 0x000000ff},
};
-static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
{0x0c00, 0x00030002, 0x00ff00ff},
{0x0c14, 0x00005252, 0x0000ffff},
{0x0c28, 0x80000000, 0xff000000},
@@ -76,7 +79,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
{0x0c00, 0x00000003, 0x000000ff},
};
-static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
{0x0204, 0x00000080, 0x000000ff},
{0x0208, 0x0000920d, 0x0000ffff},
{0x0204, 0xfc000000, 0xff000000},
@@ -106,7 +109,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
{0x03cc, 0x00000000, 0x000000ff},
};
-static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
{0x0a00, 0x00000800, 0x0000ff00},
{0x0a84, 0x00000000, 0x000000ff},
{0x0a8c, 0x00130000, 0x00ff0000},
@@ -124,7 +127,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
{0x0ac0, 0x0000008b, 0x000000ff},
};
-static struct serdes_cfg cfg_cm_c1_c2[] = {
+static const struct serdes_cfg cfg_cm_c1_c2[] = {
{0x0208, 0x00000000, 0x00000f00},
{0x0208, 0x00000000, 0x0000001f},
{0x0204, 0x00000000, 0x00040000},
@@ -185,8 +188,8 @@ static void netcp_xgbe_serdes_com_enable(void __iomem *serdes_regs)
}
}
-static void netcp_xgbe_serdes_lane_enable(
- void __iomem *serdes_regs, int lane)
+static void netcp_xgbe_serdes_lane_enable(void __iomem *serdes_regs,
+ int lane)
{
/* Set Lane Control Rate */
writel(0xe0e9e038, serdes_regs + 0x1fe0 + (4 * lane));
@@ -230,7 +233,7 @@ static int netcp_xgbe_wait_pll_locked(void __iomem *sw_regs)
cpu_relax();
} while (true);
- pr_err("XGBE serdes not locked: time out.\n");
+ pr_err("serdes not locked: time out\n");
return ret;
}
@@ -292,8 +295,7 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
u32 tmp, dlpf, tbus;
/*Get the DLPF values */
- tmp = netcp_xgbe_serdes_read_select_tbus(
- serdes_regs, lane + 1, 5);
+ tmp = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane + 1, 5);
dlpf = tmp >> 2;
@@ -302,10 +304,10 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
mdelay(1);
reg_rmw(sig_detect_reg, VAL_SH(0, 1), MASK_WID_SH(2, 1));
} else {
- tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane +
- 1, 0xe);
+ tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs,
+ lane + 1, 0xe);
- pr_debug("XGBE: CDR centered, DLPF: %4d,%d,%d.\n",
+ pr_debug("CDR centered, DLPF: %4d,%d,%d\n",
tmp >> 2, tmp & 3, (tbus >> 2) & 3);
}
}
@@ -340,13 +342,13 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
case 0:
/* if good link lock the signal detect ON! */
if (!loss && blk_lock) {
- pr_debug("XGBE PCSR Linked Lane: %d\n", i);
+ pr_debug("PCSR Linked Lane: %d\n", i);
reg_rmw(sig_detect_reg, VAL_SH(3, 1),
MASK_WID_SH(2, 1));
current_state[i] = 1;
} else if (!blk_lock) {
/* if no lock, then reset CDR */
- pr_debug("XGBE PCSR Recover Lane: %d\n", i);
+ pr_debug("PCSR Recover Lane: %d\n", i);
netcp_xgbe_serdes_reset_cdr(serdes_regs,
sig_detect_reg, i);
}
@@ -361,10 +363,10 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
break;
case 2:
- if (blk_lock)
+ if (blk_lock) {
/* Nope just noise */
current_state[i] = 1;
- else {
+ } else {
/* Lost the block lock, reset CDR if it is
* not centered and go back to sync state
*/
@@ -375,7 +377,7 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
break;
default:
- pr_err("XGBE: unknown current_state[%d] %d\n",
+ pr_err("unknown current_state[%d] %d\n",
i, current_state[i]);
break;
}
@@ -417,19 +419,19 @@ static int netcp_xgbe_serdes_check_lane(void __iomem *serdes_regs,
break;
if (lane_down[0])
- pr_debug("XGBE: detected link down on lane 0\n");
+ pr_debug("detected link down on lane 0\n");
if (lane_down[1])
- pr_debug("XGBE: detected link down on lane 1\n");
+ pr_debug("detected link down on lane 1\n");
if (++retries > 1) {
- pr_debug("XGBE: timeout waiting for serdes link up\n");
+ pr_debug("timeout waiting for serdes link up\n");
return -ETIMEDOUT;
}
mdelay(100);
} while (!link_up);
- pr_debug("XGBE: PCSR link is up\n");
+ pr_debug("PCSR link is up\n");
return 0;
}
@@ -494,7 +496,7 @@ int netcp_xgbe_serdes_init(void __iomem *serdes_regs, void __iomem *xgbe_regs)
/* read COMLANE bits 4:0 */
val = readl(serdes_regs + 0xa00);
if (val & 0x1f) {
- pr_debug("XGBE: serdes already in operation - reset\n");
+ pr_debug("serdes already in operation - reset\n");
netcp_xgbe_reset_serdes(serdes_regs);
}
return netcp_xgbe_serdes_config(serdes_regs, xgbe_regs);
^ permalink raw reply related
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: Marcelo Ricardo Leitner @ 2014-12-11 18:36 UTC (permalink / raw)
To: Jiri Benc; +Cc: vadim4j, netdev
In-Reply-To: <20141211190846.1e25e7fa@griffin>
On 11-12-2014 16:08, Jiri Benc wrote:
> 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).
Then the if() above wouldn't match. That if means to check
/proc/self/exe against the result of the path expansion. If that fails,
continue with the normal path. If it matches, it is the same binary, and
no need to re-exec itself.
> All programs should do what they are told to do, not try to outsmart
> the user.
It's not outsmarting, it's just not being dumb and doing it the proper
way. Bash itself does this twist a lot. If you just type 'echo hi', it
won't execute /bin/echo but use a built-in version. But if you write
"/bin/echo hi", it will use /bin/echo..
We could use the same idea. "ip netns exec ip" -> ellipse it and avoid
the fork/exec. But if it's cmd != "ip", execute it..
Now consider other applications that are user of this command. They will
have to implement something like:
if (this ip command has --netns argument) {
cmd="ip --netns ..."
} else {
cmd="ip netns exec ..."
}
which is ugly and inconvenient.
Marcelo
^ permalink raw reply
* RE: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Arad, Ronen @ 2014-12-11 18:47 UTC (permalink / raw)
To: Hubert Sokolowski, Roopa Prabhu, netdev@vger.kernel.org; +Cc: Jamal Hadi Salim
In-Reply-To: <6e03100b17d3e17625f1c323d94a853e.squirrel@poczta.wsisiz.edu.pl>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Hubert Sokolowski
> Sent: Thursday, December 11, 2014 8:40 AM
> To: Roopa Prabhu
> Cc: netdev@vger.kernel.org; Jamal Hadi Salim
> Subject: Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if
> ndo_fdb_dump is defined.
>
> > 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
>
I agree with Hubert on that. When a device defines its own ndo_fdb_dump it implies it wants control over the information that will be returned to user-space.
The proposed patch changes the recent status quo but it only restores it to the way it was before 5e6d243587990a588143b9da3974833649595587.
A compromise could be to also include in the same patch-set patches to the drivers that have benefited from the implicit call to ndo_dflt_fdb_dump.
-Ronen
> --
> 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: [RFC PATCH 1/3] lib: adding an Array-based Lock-Free (ALF) queue
From: David Miller @ 2014-12-11 19:15 UTC (permalink / raw)
To: brouer-H+wXaHxf7aLQT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, cl-vYTEC60ixJUAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
alexander.duyck-Re5JQEeQqe8AvxtiuMwx3w,
ast-uqk4Ao+rVK5Wk0Htik3J/w,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
rostedt-nx8X9YLhiw1AfugRpC6u6w
In-Reply-To: <20141210141512.31779.96487.stgit@dragon>
From: Jesper Dangaard Brouer <brouer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Wed, 10 Dec 2014 15:15:26 +0100
> +static inline int
> +alf_mp_enqueue(const u32 n;
> + struct alf_queue *q, void *ptr[n], const u32 n)
> +{
...
> +/* Main Multi-Consumer DEQUEUE */
> +static inline int
> +alf_mc_dequeue(const u32 n;
> + struct alf_queue *q, void *ptr[n], const u32 n)
> +{
I would seriously consider not inlining these.
^ permalink raw reply
* bind() should not return -EADDRINUSE
From: Phillip Susi @ 2014-12-11 19:19 UTC (permalink / raw)
To: netdev
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Attempting to establish a connection to a remote host using the same
local port, but a different remote port as the previous connection (
that is still in TIME_WAIT ) results in bind() returning -EADDRINUSE.
By changing the remote port, you avoid the conflict with the other
connection that is in TIME_WAIT, but since the remote port is not
known when bind() is called, it incorrectly returns -EADDRINUSE. This
check should not be done in bind(), but deferred until the remote port
is known in the call to connect(), or listen().
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
iQEcBAEBAgAGBQJUie4vAAoJENRVrw2cjl5R8CgH/jgdrBXs1Yso7XsHtrsUgO75
P8bY3Mf8eOuAkLvwbHopyGr19UZJJ+5zCQWc6bus9BRa7jSQvPCRw8fSjg3u+NXs
r9/qEqiDrYwAJqV5SwqDPGU8xGwq8v31bbT09spGIL9fABW3krBFtDHcZ/PzGA/p
UGDgELdkU7QIMKvC7FAV5RzvMXpmAI+m7iQiF936QkGZPN0AOCTea0sB+IpeidTO
/ChLd2iaEqAmS6UGh3jIQSNcTB3Ho3imHHMjTCel2v8+SgCBb2Tp3HzfmmIsGAnh
YeH+ZCNHD08c+k9yprEVjCH6fwMOo7CWzyPnSme+Ggk0L5r9oj8iz4xIeIMv43o=
=Ou3S
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: randconfig build error with next-20141210, in drivers/net/ethernet/broadcom/genet
From: David Miller @ 2014-12-11 19:22 UTC (permalink / raw)
To: jim.epost; +Cc: sfr, linux-next, linux-kernel, f.fainelli, netdev
In-Reply-To: <CA+r1ZhjxtK8BtF16Mr9jXjqvFVV7KAAFjWqfVFUibNreZwZxTw@mail.gmail.com>
From: Jim Davis <jim.epost@gmail.com>
Date: Wed, 10 Dec 2014 09:10:45 -0700
> Building with the attached random configuration file,
>
> ERROR: "fixed_phy_register"
> [drivers/net/ethernet/broadcom/genet/genet.ko] undefined!
Florian, I don't understand why FIXED_PHY is only selected in Kconfig
if the driver is statically built into the kernel.
That makes no sense at all, you should need that module regardless of
how the driver itself is enabled.
Can't we just remove the "XXX=y" in all of those silly:
select FIXED_PHY if XXX=y
expressions?
There are three such cases right now:
drivers/net/dsa/Kconfig: select FIXED_PHY if NET_DSA_BCM_SF2=y
drivers/net/ethernet/broadcom/Kconfig: select FIXED_PHY if BCMGENET=y
drivers/net/ethernet/broadcom/Kconfig: select FIXED_PHY if SYSTEMPORT=y
^ permalink raw reply
* Re: bind() should not return -EADDRINUSE
From: David Miller @ 2014-12-11 19:23 UTC (permalink / raw)
To: psusi; +Cc: netdev
In-Reply-To: <5489EE2F.6030502@ubuntu.com>
From: Phillip Susi <psusi@ubuntu.com>
Date: Thu, 11 Dec 2014 14:19:11 -0500
> Attempting to establish a connection to a remote host using the same
> local port, but a different remote port as the previous connection (
> that is still in TIME_WAIT ) results in bind() returning -EADDRINUSE.
> By changing the remote port, you avoid the conflict with the other
> connection that is in TIME_WAIT, but since the remote port is not
> known when bind() is called, it incorrectly returns -EADDRINUSE. This
> check should not be done in bind(), but deferred until the remote port
> is known in the call to connect(), or listen().
Bind has to allocate and hold from everyone else on the system the
local port at bind() time, so we cannot defer this decision.
^ permalink raw reply
* Re: [PATCH] cxgb4/csiostor: Don't use MASTER_MUST for fw_hello call
From: David Miller @ 2014-12-11 19:25 UTC (permalink / raw)
To: hariprasad
Cc: netdev, linux-scsi, JBottomley, hch, leedom, anish, nirranjan,
praveenm
In-Reply-To: <1418276503-23001-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Thu, 11 Dec 2014 11:11:43 +0530
> Remove use of calls into t4_fw_hello() with MASTER_MUST, which results in
> FW_HELLO_CMD_MASTERFORCE being set. The firmware doesn't support this and of
> course any existing PF Drivers will totally go for a toss.
>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net v2] gianfar: Fix dma check map error when DMA_API_DEBUG is enabled
From: David Miller @ 2014-12-11 19:27 UTC (permalink / raw)
To: haokexin; +Cc: netdev, claudiu.manoil
In-Reply-To: <1418278121-20209-1-git-send-email-haokexin@gmail.com>
From: Kevin Hao <haokexin@gmail.com>
Date: Thu, 11 Dec 2014 14:08:41 +0800
> 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:
...
> 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>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
> v2: Just update the RX path to reuse the original skb when dma mapping error
> occurs as suggested by David.
Looks good, applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 1/1] net/macb: add TX multiqueue support for gem
From: Thomas Petazzoni @ 2014-12-11 19:31 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: nicolas.ferre, davem, linux-arm-kernel, netdev, soren.brinkmann,
linux-kernel
In-Reply-To: <87a3098203ee6eaa7a60607713a293d3258e2b58.1418291637.git.cyrille.pitchen@atmel.com>
Dear Cyrille Pitchen,
On Thu, 11 Dec 2014 11:16:51 +0100, Cyrille Pitchen wrote:
> +#define GEM_ISR1 0x0400
> +#define GEM_ISR2 0x0404
> +#define GEM_ISR3 0x0408
> +#define GEM_ISR4 0x040c
> +#define GEM_ISR5 0x0410
> +#define GEM_ISR6 0x0414
> +#define GEM_ISR7 0x0418
What about doing instead:
#define GEM_ISR(q) ((q) == 0 ? MACB_ISR : 0x400 + (q) << 2)
And ditto for all other registers, which will save a lot of boring repeated code.
If you do that, then you can avoid the following fields in the
macb_queue structure:
+ unsigned int ISR;
+ unsigned int IER;
+ unsigned int IDR;
+ unsigned int IMR;
+ unsigned int TBQP;
And the not very pleasant calculation of those offsets:
+ bp->queues[0].bp = bp;
+ bp->queues[0].ISR = MACB_ISR;
+ bp->queues[0].IER = MACB_IER;
+ bp->queues[0].IDR = MACB_IDR;
+ bp->queues[0].IMR = MACB_IMR;
+ bp->queues[0].TBQP = MACB_TBQP;
+ for (q = 1, queue = &bp->queues[1]; q < MACB_MAX_QUEUES; ++q) {
+ if (!(queue_mask & (1 << q)))
+ continue;
+
+ queue->bp = bp;
+ queue->ISR = (q-1) * sizeof(u32) + GEM_ISR1;
+ queue->IER = (q-1) * sizeof(u32) + GEM_IER1;
+ queue->IDR = (q-1) * sizeof(u32) + GEM_IDR1;
+ queue->IMR = (q-1) * sizeof(u32) + GEM_IMR1;
+ queue->TBQP = (q-1) * sizeof(u32) + GEM_TBQP1;
+ queue++;
+ }
You replace the ISR, IER, IDR, IMR and TBQP by an "id" field in
macb_queue, which contains the queue number, and then change your:
+#define queue_readl(queue, reg) \
+ __raw_readl((queue)->bp->regs + queue->reg)
+#define queue_writel(queue, reg, value) \
+ __raw_writel((value), (queue)->bp->regs + queue->reg)
to
+#define queue_readl(queue, reg) \
+ __raw_readl((queue)->bp->regs + reg((queue)->id))
+#define queue_writel(queue, reg, value) \
+ __raw_writel((value), (queue)->bp->regs + reg((queue)->id)
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [net] gre: fix the inner mac header in nbma gre tunnels xmit path
From: David Miller @ 2014-12-11 19:36 UTC (permalink / raw)
To: timo.teras; +Cc: netdev, therbert, alexander.h.duyck
In-Reply-To: <1418282079-30245-1-git-send-email-timo.teras@iki.fi>
From: Timo Teräs <timo.teras@iki.fi>
Date: Thu, 11 Dec 2014 09:14:39 +0200
> @@ -266,6 +262,7 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
> * to gre header.
> */
> skb_pull(skb, tunnel->hlen + sizeof(struct iphdr));
> + skb_reset_mac_header(mac);
Please explain to me how this compiles, let alone be functionally
tested.
^ permalink raw reply
* Re: [PATCH v2 net] be2net: Export tunnel offloads only when a VxLAN tunnel is created
From: David Miller @ 2014-12-11 19:44 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev
In-Reply-To: <1418286287-19738-1-git-send-email-sathya.perla@emulex.com>
From: Sathya Perla <sathya.perla@emulex.com>
Date: Thu, 11 Dec 2014 03:24:47 -0500
> From: Sriharsha Basavapatna <sriharsha.basavapatna@emulex.com>
>
> The encapsulated offload flags shouldn't be unconditionally exported
> to the stack. The stack expects offloading to work across all tunnel
> types when those flags are set. This would break other tunnels (like
> GRE) since be2net currently supports tunnel offload for VxLAN only.
>
> Also, with VxLANs Skyhawk-R can offload only 1 UDP dport. If more
> than 1 UDP port is added, we should disable offloads in that case too.
>
> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>
> v2 changes: fix a bad indentation pointed out by Dave M.
Applied, thanks.
^ permalink raw reply
* Re: [net] gre: fix the inner mac header in nbma gre tunnels xmit path
From: Timo Teras @ 2014-12-11 19:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev, therbert, alexander.h.duyck
In-Reply-To: <20141211.143627.1516156524809595238.davem@davemloft.net>
On Thu, 11 Dec 2014 14:36:27 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> From: Timo Teräs <timo.teras@iki.fi>
> Date: Thu, 11 Dec 2014 09:14:39 +0200
>
> > @@ -266,6 +262,7 @@ static netdev_tx_t ipgre_xmit(struct sk_buff
> > *skb,
> > * to gre header.
> > */
> > skb_pull(skb, tunnel->hlen + sizeof(struct iphdr));
> > + skb_reset_mac_header(mac);
>
> Please explain to me how this compiles, let alone be functionally
> tested.
Sorry. I made the change twice; once on the build box. And again on my
git checkout on work station. Must've been on seriously coffee deprived
state.
Should be obviously:
+ skb_reset_mac_header(skb);
Is there other comments on it?
Or shall I resend?
^ permalink raw reply
* Re: [PATCH V2 net-next 00/10] mlx4 driver update
From: David Miller @ 2014-12-11 19:48 UTC (permalink / raw)
To: ogerlitz; +Cc: netdev, matanb, amirv, talal, jackm
In-Reply-To: <1418288280-334-1-git-send-email-ogerlitz@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Thu, 11 Dec 2014 10:57:50 +0200
> This series from Matan, Jenny, Dotan and myself is mostly about adding
> support to a new performance optimized flow steering mode (patches 4-10).
>
> The 1st two patches are small fixes (one for VXLAN and one for SRIOV),
> and the third patch is a fix to avoid hard-lockup situation when many
> (hunderds) processes holding user-space QPs/CQs get events.
>
> Matan and Or.
Series applied, thanks.
^ 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