* Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
From: Greg Kroah-Hartman @ 2014-08-25 17:50 UTC (permalink / raw)
To: KY Srinivasan
Cc: netdev@vger.kernel.org, Jason Wang, Haiyang Zhang,
linux-kernel@vger.kernel.org, Daniel Borkmann,
devel@linuxdriverproject.org, David S. Miller, Dan Carpenter,
Sitsofe Wheeler
In-Reply-To: <cf94fd58947d4c5aa6d0d369c1126d7f@BY2PR0301MB0711.namprd03.prod.outlook.com>
On Mon, Aug 25, 2014 at 05:34:27PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > Sent: Monday, August 25, 2014 2:37 AM
> > To: Sitsofe Wheeler
> > Cc: KY Srinivasan; Greg Kroah-Hartman; Jason Wang; linux-
> > kernel@vger.kernel.org; David S. Miller; Daniel Borkmann;
> > netdev@vger.kernel.org; devel@linuxdriverproject.org; Haiyang Zhang
> > Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
> >
> > The code here is:
> >
> > drivers/hv/channel.c
> > 460 BUG_ON(ret != 0);
> > 461 t = wait_for_completion_timeout(&info->waitevent, 5*HZ);
> > 462 BUG_ON(t == 0);
> >
> > So it calls BUG_ON() if the teardown takes more than 5 seconds. It's most
> > likely that there is a race condition somewhere. It's also possible that it's just
> > taking longer than 5 seconds for some odd reason and the bug would go
> > away if we raised it to 60 seconds.
> >
> > BUG_ON() after 5 seconds seems like a very aggressive thing.
>
> Dan,
>
> I am going to audit all BUG_ON() instances.
Please remove them all, no kernel driver should ever crash the kernel
and not give a user a chance to recover :(
thanks,
greg k-h
^ permalink raw reply
* RE: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
From: KY Srinivasan @ 2014-08-25 17:34 UTC (permalink / raw)
To: Dan Carpenter, Sitsofe Wheeler
Cc: Greg Kroah-Hartman, Jason Wang, linux-kernel@vger.kernel.org,
David S. Miller, Daniel Borkmann, netdev@vger.kernel.org,
devel@linuxdriverproject.org, Haiyang Zhang
In-Reply-To: <20140825093648.GB5046@mwanda>
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, August 25, 2014 2:37 AM
> To: Sitsofe Wheeler
> Cc: KY Srinivasan; Greg Kroah-Hartman; Jason Wang; linux-
> kernel@vger.kernel.org; David S. Miller; Daniel Borkmann;
> netdev@vger.kernel.org; devel@linuxdriverproject.org; Haiyang Zhang
> Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
>
> The code here is:
>
> drivers/hv/channel.c
> 460 BUG_ON(ret != 0);
> 461 t = wait_for_completion_timeout(&info->waitevent, 5*HZ);
> 462 BUG_ON(t == 0);
>
> So it calls BUG_ON() if the teardown takes more than 5 seconds. It's most
> likely that there is a race condition somewhere. It's also possible that it's just
> taking longer than 5 seconds for some odd reason and the bug would go
> away if we raised it to 60 seconds.
>
> BUG_ON() after 5 seconds seems like a very aggressive thing.
Dan,
I am going to audit all BUG_ON() instances.
Thanks,
K. Y
>
> regards,
> dan carpenter
^ permalink raw reply
* RE: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
From: KY Srinivasan @ 2014-08-25 17:29 UTC (permalink / raw)
To: Richard Weinberger, Sitsofe Wheeler
Cc: Haiyang Zhang, Greg Kroah-Hartman, devel@linuxdriverproject.org,
linux-kernel@vger.kernel.org, Jason Wang, Daniel Borkmann,
David S. Miller, netdev@vger.kernel.org
In-Reply-To: <53FB511E.5040906@nod.at>
> -----Original Message-----
> From: Richard Weinberger [mailto:richard@nod.at]
> Sent: Monday, August 25, 2014 8:07 AM
> To: KY Srinivasan; Sitsofe Wheeler
> Cc: Haiyang Zhang; Greg Kroah-Hartman; devel@linuxdriverproject.org;
> linux-kernel@vger.kernel.org; Jason Wang; Daniel Borkmann; David S. Miller;
> netdev@vger.kernel.org
> Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
>
> Am 25.08.2014 16:53, schrieb KY Srinivasan:
> >
> >
> >> -----Original Message-----
> >> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> >> Sent: Monday, August 25, 2014 3:48 AM
> >> To: Sitsofe Wheeler
> >> Cc: Haiyang Zhang; KY Srinivasan; Greg Kroah-Hartman;
> >> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; Jason
> >> Wang; Daniel Borkmann; David S. Miller; netdev@vger.kernel.org
> >> Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing
> >> MTU
> >>
> >> via
> >>
> >>
> >> On Wed, Aug 20, 2014 at 5:41 AM, Sitsofe Wheeler <sitsofe@gmail.com>
> >> wrote:
> >>> Aug 20 04:04:41 ubuntuhv kernel: [ 9.230399] random: nonblocking pool
> is
> >> initialized
> >>> Aug 20 04:04:41 ubuntuhv kernel: [ 10.338487] EXT4-fs (sda1): re-
> >> mounted. Opts: errors=remount-ro
> >>> Aug 20 04:04:41 ubuntuhv kernel: [ 11.099094] hv_storvsc vmbus_0_1:
> >> cmd 0x85 scsi status 0x2 srb status 0x6
> >>> Aug 20 04:04:41 ubuntuhv kernel: [ 11.099901] hv_storvsc vmbus_0_1:
> >> cmd 0x85 scsi status 0x2 srb status 0x6
> >>> Aug 20 04:04:43 ubuntuhv kernel: [ 12.999830] psmouse serio1:
> trackpoint:
> >> IBM TrackPoint firmware: 0x01, buttons: 0/0
> >>> Aug 20 03:55:47 ubuntuhv kernel: [ 13.003659] input: TPPS/2 IBM
> >> TrackPoint as /devices/platform/i8042/serio1/input/input4
> >>> Aug 20 03:57:28 ubuntuhv kernel: [ 113.711832] hv_netvsc vmbus_0_14:
> >>> net device safe to remove Aug 20 03:57:28 ubuntuhv kernel: [
> >>> 113.713882] hv_netvsc: hv_netvsc channel opened successfully Aug 20
> >>> 03:57:29 ubuntuhv kernel: [ 114.961312] hv_netvsc vmbus_0_14: Send
> >>> section size: 6144, Section count:2560 Aug 20 03:57:29 ubuntuhv
> >>> kernel: [ 114.962711] hv_netvsc vmbus_0_14: Device MAC
> >>> 00:15:5d:6f:02:af link state up Aug 20 03:57:34 ubuntuhv kernel: [
> >>> 120.027718] hv_netvsc vmbus_0_14: net device safe to remove Aug 20
> >>> 03:57:34 ubuntuhv kernel: [ 120.030047] hv_netvsc: hv_netvsc
> >>> channel opened successfully Aug 20 03:57:34 ubuntuhv kernel: [
> >>> 120.035422]
> >> hv_netvsc vmbus_0_14 eth0: unable to establish receive buffer's gpadl
> >> Aug
> >> 20 03:57:34 ubuntuhv kernel: [ 120.039778] hv_netvsc vmbus_0_14 eth0:
> >> unable to connect to NetVSP - 4 Aug 20 03:57:34 ubuntuhv kernel: [
> >> 120.039818] ------------[ cut here ]------------ Aug 20 03:57:34 ubuntuhv
> kernel:
> >> [ 120.039832] kernel BUG at drivers/hv/channel.c:504!
> >>
> >> This is one is also a rude BUG_ON:
> >> ret = vmbus_post_msg(msg, sizeof(struct
> >> vmbus_channel_close_channel));
> >>
> >> BUG_ON(ret != 0);
> >>
> >> vmbus_post_msg() hv_post_message() can easily return !0.
> >> i.e. if this kmalloc() fails:
> >> addr = (unsigned long)kmalloc(sizeof(struct aligned_input),
> >> GFP_ATOMIC);
> >> if (!addr)
> >> return -ENOMEM;
> >
> > I will submit a patch to handle this case. I suspect though that the
> > original ASSERT at channel.c (line 504) is not related to kmalloc failing in
> vmbus_post_msg(). I will also look at that issue as well.
>
> I'm sure you can replace most BUG_ON() by a WARN_ON().
> Also print more details why the assertion does not hold.
The cases where I have intended to use BUG_ON are only cases where there was
reasonable way to roll back from the failure (barring bugs such as what we are discussing hre).
I will audit all BUG_ON calls and also print details to help debug.
Thank you,
K. Y
>
> Thanks,
> //richard
^ permalink raw reply
* Re: [PATCH V4 3/8] namespaces: expose ns instance serial numbers in proc
From: Andy Lutomirski @ 2014-08-25 16:50 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Serge E. Hallyn, Eric W. Biederman,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Linux API, Richard Guy Briggs,
netdev
In-Reply-To: <53FB673F.8070200-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
On Mon, Aug 25, 2014 at 9:41 AM, Nicolas Dichtel
<nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:
> Le 25/08/2014 18:13, Andy Lutomirski a écrit :
>
>> On Mon, Aug 25, 2014 at 8:43 AM, Nicolas Dichtel
>> <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> Le 25/08/2014 16:04, Andy Lutomirski a écrit :
>>>
>>>> On Aug 25, 2014 6:30 AM, "Nicolas Dichtel" <nicolas.dichtel@6wind.com>
>>>> wrote:
>>>>>>
>>>>>>
>>>>>> CRIU wants to save the complete state of a namespace and then restore
>>>>>> it. For that to work, any information exposed to things in the
>>>>>> namespace *cannot* be globally unique or unique per boot, since CRIU
>>>>>> needs to arrange for that information to match whatever it was when
>>>>>> CRIU saved it.
>>>>>
>>>>>
>>>>>
>>>>> How are ifindex of network devices managed? These ifindexes are unique
>>>>> per boot,
>>>>> thus can change depending on the order in which netdev are created.
>>>>> These ifindexes are unique per boot and exposed to userspace ...
>>>>>
>>>>
>>>> This does not appear to be true.
>>>>
>>>> $ sudo unshare --net
>>>> # ip link add veth0 type veth peer name veth1
>>>> # ip link
>>>> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group
>>>> default
>>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>>> 2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>>>> DEFAULT group default qlen 1000
>>>> link/ether 06:0d:59:c7:a6:a8 brd ff:ff:ff:ff:ff:ff
>>>> 3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>>>> DEFAULT group default qlen 1000
>>>> link/ether b2:5c:8b:f2:12:28 brd ff:ff:ff:ff:ff:ff
>>>> # logout
>>>> $ ip link
>>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
>>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>>> 3: em1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
>>>> state DOWN qlen 1000
>>>>
>>> I've probably misunderstood what you're trying to say. ifindexes are
>>> unique
>>> per
>>> boot and per netns.
>>
>>
>> I think we both misunderstood each other. The ifindexes are unique
>> *per netns*, which means that, if you're unprivileged in a netns,
>> global information doesn't leak to you. I think this is good.
>
> Ok, I agree. I think audit daemons are always running under privileged
> users.
>
>
>>
>>>>
>>>> Let me try again, with emphasis in the right place.
>>>>
>>>> I think that *code running in a namespace* has no business even
>>>> knowing a unique identity of *that namespace* from the perspective of
>>>> the host.
>>>>
>>>> In your example, if there's a veth device between netns A and netns B,
>>>> then code *in netns A* has no business knowing the identity of its
>>>> veth peer if its peer (B) is a sibling or ancestor. It also IMO has
>>>> no business knowing the identity of its own netns (A) other than as
>>>> "my netns".
>>>
>>>
>>> I do not agree (see the example below).
>>>
>>>
>>>>
>>>> If A and B are siblings, then their parent needs to know where that
>>>> veth device goes, but I think this is already the case to a sufficient
>>>> extent today.
>>>
>>>
>>> I'm not aware of a hierarchy between netns. A daemon should be able to
>>> got the full network configuration, even if it's started when this
>>> configuration
>>> is already applied, ie even if it doesn't know what happen before it
>>> starts.
>>>
>>
>> I don't know exactly which namespaces have an explicit hierarchy, but
>> there is certainly a hierarchy of *user* namespaces, and network
>> namespaces live in user namespaces, so they at least have somewhat of
>> a hierarchy.
>>
>>>
>>>>
>>>> I feel like this discussion is falling into a common trap of new API
>>>> discussions. Can one of you who wants this API please articulate,
>>>> with a reasonably precise example, what it is that you want to do, why
>>>> you can't easily do it already, and how this API helps? I currently
>>>> understand how the API creates problems, but I don't understand how it
>>>> solves any problems, and I will NAK it (and I suspect that Eric will,
>>>> too, which is pretty much fatal) unless that changes.
>>>
>>>
>>> What I'm trying to solve is to have full info in netlink messages sent by
>>> the
>>> kernel, thus beeing able to identify a peer netns (and this is close from
>>> what
>>> audit guys are trying to have). Theorically, messages sent by the kernel
>>> can
>>> be
>>> reused as is to have the same configuration. This is not the case with
>>> x-netns
>>> devices. Here is an example, with ip tunnels:
>>>
>>> $ ip netns add 1
>>> $ ip link add ipip1 type ipip remote 10.16.0.121 local 10.16.0.249 dev
>>> eth0
>>> $ ip -d link ls ipip1
>>> 8: ipip1@eth0: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode
>>> DEFAULT group default
>>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
>>> ipip remote 10.16.0.121 local 10.16.0.249 dev eth0 ttl inherit
>>> pmtudisc
>>> $ ip link set ipip1 netns 1
>>> $ ip netns exec 1 ip -d link ls ipip1
>>> 8: ipip1@tunl0: <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
>>> mode DEFAULT group default
>>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
>>> ipip remote 10.16.0.121 local 10.16.0.249 dev tunl0 ttl inherit
>>> pmtudisc
>>>
>>> Now informations got with 'ip link' are wrong and incomplete:
>>> - the link dev is now tunl0 instead of eth0, because we only got an
>>> ifindex
>>> from the kernel without any netns informations.
>>> - the encapsulation addresses are not part of this netns but the user
>>> doesn't
>>> known that (still because netns info is missing). These IPv4
>>> addresses
>>> may
>>> exist into this netns.
>>> - it's not possible to create the same netdevice with these infos.
>>>
>>
>> Aha. That's a genuine problem.
>>
>> Perhaps we need a concept of which netnses should be able to see each
>> other.
>
> Yes, I agree. This is not required for all netns, only a subset of netns
> should
>
> be able to see each other.
>
>>
>> I think I would be okay with a somewhat different outcome from your
>> example:
>>
>> $ ip netns exec 1 ip -d link ls ipip1
>> 8: ipip1@[unknown device in another namespace]:
>> <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
>>
>> I think this outcome is mandatory if netns 1 lives in a subsidiary
>> user namespace.
>
> Yes.
>
>
>>
>> Certainly, if you do the 'ip link' in the original namespace, I agree
>> that this should work.
>
> And yes :)
>
> I will update my previous proposal
> (http://thread.gmane.org/gmane.linux.network/315933/focus=321753)
> to allow to get an id for a peer netns only when the user namespace is the
> same.
>
I think it should work if the peer userns is the same or a descendent.
I also wonder whether the peer's ifindex should be suppressed if peer
userns is not the same or a descendent.
Now you just have to get Eric to be happy with the id allocation. :)
This may be nontrivial.
>
>>
>> For most namespace types, this all works transparently, since
>> everything has an real identity all the way up the hierarchy. Network
>> namespaces are different.
>>
>> I don't think that exposing serial numbers in /proc is a good
>> solution, both for the reasons already described and because I don't
>> think that iproute2 should need to muck around with /proc to function
>
> A netlink API is probably enough. But it will help only for the network
> problem, not for audit. I was hoping to find a common solution.
I still don't understand why audit needs anything beyond the audit
part of this patch set. I have no problem with audit seeing that
migrated/restored namespaces are really brand-new namespaces, as long
as the code in those namespaces isn't exposed to it.
>
>
>> correctly. Eric, any clever ideas here? Do we need fancier netlink
>> messages for this?
>>
>> --Andy
>>
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jamal Hadi Salim @ 2014-08-25 16:48 UTC (permalink / raw)
To: Thomas Graf
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, John Fastabend,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Jiri Pirko,
roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20140825145449.GB30140-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>
On 08/25/14 10:54, Thomas Graf wrote:
> On 08/24/14 at 11:15am, Jamal Hadi Salim wrote:
> Let's keep vendors out of this discussion.
The API is from a vendor. It is clearly labelled as an OF API.
It covers well abstracting that vendors SDK to enable OF. That
is relevant info.
If it covers all other vendors (which is where
the quark handling comes in), I will be fine with it.
I dont believe it does.
> That is simply not the case. The fact that John is using this model
> to replace the flow director ioctl API should prove this.
depends what NIC classifier John is mapping to. The Intels have
about 4-5 different types of classifier on different hardware
with different interfaces.
If it is a "flow type" - yes. I think you could wing-in the
RSS(and somehow announce you cant handle UDP). You may be able
to tie in RSS.
I am not sure about VMDQ; neither am i sure about what happens
when you need to deal with a combination of 2 or more classifiers
which i believe is part of the lookups in such hardware.
So that aside:
If you are telling me John is going to also map the L2 fdb here we are
going to have a strong disagreement.
And back to my earlier arguement:
allow for multiple classifiers to be expressed not THE ONE.
If i wanted to support CLASSIFIER_RSS from tc i could write one
and i can use tc to configure it. Or i could write one for nftables.
In general i probably should be able to wing it with some small
acrobatics.
> There is not a single bit specific to OpenFlow and there is absolutely
> no awareness of OF within the kernel in OVS.
>
The API is for OF support in a vendor ASIC.
>> fields in the packet. That is not the challenge for such an
>> API. The challenge is dealing with the quarks.
>> Some chips implement FIB and NH conjoined; others implement
>> them separately.
>> I dont see how this is even being remotely touched on.
>
> First of all, that sounds like exactly like something that should
> be handled in the driver specific portion of the API. Secondly,
> can you provide additional information on these specific pieces of
> hardware so we take it into account?
>
I gave a simple example.
There are a hell more quarks than that.
There are cases where there are multiple tables in terms of net masks
etc.
Yes, this should be handled in the driver. The input is the route
message we already specify and not some XXX_Flow_XXx struct.
> Realistically there will only be a handful, maybe something
> like:
>
> flow_insert / flow_remove
> p4_add / p4_remove
> [...]
>
> Maybe you can share some information the specific API you have
> in mind?
>
I would be tagging along with you guys for flows if you:
a) allow for different classifiers. This allows me to implement
u32 and offload it.
b) different actions (I think this part is not controversial, you
seem to be having it already).
c) stay out of L2/3. We know how to do this already. We have
representative data structures that *completely* define those.
> Agreed, I don't think anybody expects anything else.
>
I understand intent may be that. That is not the reality
when you start at OF-DPA as the api.
>> Lets start with hardware abstraction. Lets map to existing Linux APIs
>> and then see where some massaging maybe needed.
>
> That's what's being done. HW offload is being mapped to OVS and
> to an existing ioctl interface. Those are existing Linux APIs.
> Can you explain why swdev as proposed is not suitable for the
> other existing Linux APIs? They don't *have* to use the flow_insert(),
> they are free to exted the API to represent more generic programmable
> hardware.
>
I would like XXX_flow_XXX to allow for multiple types of classifiers.
nftables may express one and the driver which is capable offload it.
>> This abstraction gives OVS 1-1 mapping which is something i object to.
>> You want to penalize me for the sake of getting the OVS api in place?
>
> I don't understand this.
>
Refer to my comments earlier.
>> Beginning with flows and laying claim to that one would be able to
>> cover everything is non-starter.
>
> Nobody claims that. In fact, I'm very interested in seeing the API
> extended for non flow based models. I'm actually convinced that flow
> based models are not the ultimate answers on HW level but a vast majority
> of hardware understands some form of protocol aware exact match or
> wildcard filters of limited capacity. This category of hardware is
> being addressed with the flow_insert() API.
>
And make that also take input the classifier type.
>> There are some cases where that approach doesnt make sense:
>> example if i wanted to specify a string classifier etc.
>> But if we are talking packet header classifier - it is flexible.
>> There are also good reasons to specify a universal 5 tuple classifier.
>> As there are good reasons to specify your latest OF classifier.
>> But that OF classifier being the starting point is not pragmatic.
>
> So you agree that at least on the driver level some form of ntuple
> awareness must be given because the hardware has limited capabilities.
Yes, there is a classifier *type* where the 15 tuples makes sense.
> This is exactly what flow_insert() is, it is a generic ntuple
> classifier which can implement a subset of the 15 tuple in HW. So
> instead of adding a separate NDO for each fixed tuple, a generic
> NDO can handle the different levels of offloads. Very similar to how
> the xmit to the NIC can handle various protocol offloads already.
>
> What is being proposed is a generic ntuple with masking support to
> describe filtering needs. What is missing is a capabilities reporting
> channel so API users can know in advance what is supported to
> implement partial offloads.
>
The 15 tuple itself needs to be one-of several classifiers.
Creating a univesal classifier is problematic. Look at tc classifier
approach (which i know you understand well).
Sorry -I am on time constraint and may not be as responsive.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH V4 3/8] namespaces: expose ns instance serial numbers in proc
From: Nicolas Dichtel @ 2014-08-25 16:41 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Serge E. Hallyn, Eric W. Biederman,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Linux API, Richard Guy Briggs,
netdev
In-Reply-To: <CALCETrWHrWhm89B5s=pLt_9eTx3ZF8ifA6y6CwknWaWU7dp=sQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Le 25/08/2014 18:13, Andy Lutomirski a écrit :
> On Mon, Aug 25, 2014 at 8:43 AM, Nicolas Dichtel
> <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:
>> Le 25/08/2014 16:04, Andy Lutomirski a écrit :
>>
>>> On Aug 25, 2014 6:30 AM, "Nicolas Dichtel" <nicolas.dichtel@6wind.com>
>>> wrote:
>>>>>
>>>>> CRIU wants to save the complete state of a namespace and then restore
>>>>> it. For that to work, any information exposed to things in the
>>>>> namespace *cannot* be globally unique or unique per boot, since CRIU
>>>>> needs to arrange for that information to match whatever it was when
>>>>> CRIU saved it.
>>>>
>>>>
>>>> How are ifindex of network devices managed? These ifindexes are unique
>>>> per boot,
>>>> thus can change depending on the order in which netdev are created.
>>>> These ifindexes are unique per boot and exposed to userspace ...
>>>>
>>>
>>> This does not appear to be true.
>>>
>>> $ sudo unshare --net
>>> # ip link add veth0 type veth peer name veth1
>>> # ip link
>>> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group
>>> default
>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>> 2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>>> DEFAULT group default qlen 1000
>>> link/ether 06:0d:59:c7:a6:a8 brd ff:ff:ff:ff:ff:ff
>>> 3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>>> DEFAULT group default qlen 1000
>>> link/ether b2:5c:8b:f2:12:28 brd ff:ff:ff:ff:ff:ff
>>> # logout
>>> $ ip link
>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>> 3: em1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
>>> state DOWN qlen 1000
>>>
>> I've probably misunderstood what you're trying to say. ifindexes are unique
>> per
>> boot and per netns.
>
> I think we both misunderstood each other. The ifindexes are unique
> *per netns*, which means that, if you're unprivileged in a netns,
> global information doesn't leak to you. I think this is good.
Ok, I agree. I think audit daemons are always running under privileged users.
>
>>>
>>> Let me try again, with emphasis in the right place.
>>>
>>> I think that *code running in a namespace* has no business even
>>> knowing a unique identity of *that namespace* from the perspective of
>>> the host.
>>>
>>> In your example, if there's a veth device between netns A and netns B,
>>> then code *in netns A* has no business knowing the identity of its
>>> veth peer if its peer (B) is a sibling or ancestor. It also IMO has
>>> no business knowing the identity of its own netns (A) other than as
>>> "my netns".
>>
>> I do not agree (see the example below).
>>
>>
>>>
>>> If A and B are siblings, then their parent needs to know where that
>>> veth device goes, but I think this is already the case to a sufficient
>>> extent today.
>>
>> I'm not aware of a hierarchy between netns. A daemon should be able to
>> got the full network configuration, even if it's started when this
>> configuration
>> is already applied, ie even if it doesn't know what happen before it starts.
>>
>
> I don't know exactly which namespaces have an explicit hierarchy, but
> there is certainly a hierarchy of *user* namespaces, and network
> namespaces live in user namespaces, so they at least have somewhat of
> a hierarchy.
>
>>
>>>
>>> I feel like this discussion is falling into a common trap of new API
>>> discussions. Can one of you who wants this API please articulate,
>>> with a reasonably precise example, what it is that you want to do, why
>>> you can't easily do it already, and how this API helps? I currently
>>> understand how the API creates problems, but I don't understand how it
>>> solves any problems, and I will NAK it (and I suspect that Eric will,
>>> too, which is pretty much fatal) unless that changes.
>>
>> What I'm trying to solve is to have full info in netlink messages sent by
>> the
>> kernel, thus beeing able to identify a peer netns (and this is close from
>> what
>> audit guys are trying to have). Theorically, messages sent by the kernel can
>> be
>> reused as is to have the same configuration. This is not the case with
>> x-netns
>> devices. Here is an example, with ip tunnels:
>>
>> $ ip netns add 1
>> $ ip link add ipip1 type ipip remote 10.16.0.121 local 10.16.0.249 dev eth0
>> $ ip -d link ls ipip1
>> 8: ipip1@eth0: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode
>> DEFAULT group default
>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
>> ipip remote 10.16.0.121 local 10.16.0.249 dev eth0 ttl inherit pmtudisc
>> $ ip link set ipip1 netns 1
>> $ ip netns exec 1 ip -d link ls ipip1
>> 8: ipip1@tunl0: <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
>> mode DEFAULT group default
>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
>> ipip remote 10.16.0.121 local 10.16.0.249 dev tunl0 ttl inherit pmtudisc
>>
>> Now informations got with 'ip link' are wrong and incomplete:
>> - the link dev is now tunl0 instead of eth0, because we only got an ifindex
>> from the kernel without any netns informations.
>> - the encapsulation addresses are not part of this netns but the user
>> doesn't
>> known that (still because netns info is missing). These IPv4 addresses
>> may
>> exist into this netns.
>> - it's not possible to create the same netdevice with these infos.
>>
>
> Aha. That's a genuine problem.
>
> Perhaps we need a concept of which netnses should be able to see each other.
Yes, I agree. This is not required for all netns, only a subset of netns should
be able to see each other.
>
> I think I would be okay with a somewhat different outcome from your example:
>
> $ ip netns exec 1 ip -d link ls ipip1
> 8: ipip1@[unknown device in another namespace]:
> <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
>
> I think this outcome is mandatory if netns 1 lives in a subsidiary
> user namespace.
Yes.
>
> Certainly, if you do the 'ip link' in the original namespace, I agree
> that this should work.
And yes :)
I will update my previous proposal
(http://thread.gmane.org/gmane.linux.network/315933/focus=321753)
to allow to get an id for a peer netns only when the user namespace is the same.
>
> For most namespace types, this all works transparently, since
> everything has an real identity all the way up the hierarchy. Network
> namespaces are different.
>
> I don't think that exposing serial numbers in /proc is a good
> solution, both for the reasons already described and because I don't
> think that iproute2 should need to muck around with /proc to function
A netlink API is probably enough. But it will help only for the network
problem, not for audit. I was hoping to find a common solution.
> correctly. Eric, any clever ideas here? Do we need fancier netlink
> messages for this?
>
> --Andy
>
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jamal Hadi Salim @ 2014-08-25 16:15 UTC (permalink / raw)
To: Thomas Graf
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, John Fastabend,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w, ogerlitz,
ben-/+tVBieCtBitmTQ+vhA3Yw, buytenh-OLH4Qvv75CYX/NnBR394Jw,
Jiri Pirko, roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, Neil Horman, netdev,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, dborkman,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <20140825141754.GA30140-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>
On 08/25/14 10:17, Thomas Graf wrote:
> On 08/25/14 at 09:53am, Jamal Hadi Salim wrote:
> fdb_add() *is* flow based. At least in my understanding, the whole
> point here is to extend the idea of fdb_add() and make it understand
> L2-L4 in a more generic way for the most common protocols.
>
> The reason fdb_add() is not reused is because it is Netlink specific
> and only suitable for User -> HW offload. Kernel -> HW offload is
> technically possible but not clean.
>
I dont think we have a problem handling any of this today.
> The only reason swdev is needed at all is to represent the port model
> and to allow for non flow based models built on top of the same
> hardware abstraction. I see now reason why br_fdb cannot be represented
> through swdev as soon as the code is stable.
>
This is where our (shall i say strong) disagreement is.
I think you will find it non-trivial to show me how you can
actually take the simple L2 bridge and map it to a "flow".
Since your starting point is "everything can be represented via a flow
and some table" - we are at a crosspath.
> The point I was trying to make earlier is that it is very hard to
> program both protocol aware and generic filtering hardware through
> a single NDO. It will make the driver specific part complex.
>
The tc filter API seems to be doing just that.
You have different types of classifiers - the h/w may not be able
to support some classifier types - but that is a capability discovery
challenge.
> If you are saying we need yet another classifier model in the kernel
> then I'm not sure that is needed in the presence of cls/act, iptables,
> and nftables. They seem suitable to represent non flow based models
> and I see nothing that would prevent an offload through swdev for them.
>
I am saying two things:
1) There are a few "fundamental" interfaces; L2 and L3 being some.
Add crypto offload and a few i mentioned in my presentation. We
know how to do those. example; there is nothing i cant do with
the rtmsg that is L3. or the fdb/port/vlan filter for L2.
This flow thing should stay out of those.
2) The flow thing should allow a variety of classifiers to be
handled. Again capability discovery would take care of differences.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH V4 3/8] namespaces: expose ns instance serial numbers in proc
From: Andy Lutomirski @ 2014-08-25 16:13 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Linux API, Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Eric W. Biederman, netdev
In-Reply-To: <53FB59A3.5030804-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
On Mon, Aug 25, 2014 at 8:43 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> Le 25/08/2014 16:04, Andy Lutomirski a écrit :
>
>> On Aug 25, 2014 6:30 AM, "Nicolas Dichtel" <nicolas.dichtel@6wind.com>
>> wrote:
>>>>
>>>> CRIU wants to save the complete state of a namespace and then restore
>>>> it. For that to work, any information exposed to things in the
>>>> namespace *cannot* be globally unique or unique per boot, since CRIU
>>>> needs to arrange for that information to match whatever it was when
>>>> CRIU saved it.
>>>
>>>
>>> How are ifindex of network devices managed? These ifindexes are unique
>>> per boot,
>>> thus can change depending on the order in which netdev are created.
>>> These ifindexes are unique per boot and exposed to userspace ...
>>>
>>
>> This does not appear to be true.
>>
>> $ sudo unshare --net
>> # ip link add veth0 type veth peer name veth1
>> # ip link
>> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group
>> default
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>> DEFAULT group default qlen 1000
>> link/ether 06:0d:59:c7:a6:a8 brd ff:ff:ff:ff:ff:ff
>> 3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
>> DEFAULT group default qlen 1000
>> link/ether b2:5c:8b:f2:12:28 brd ff:ff:ff:ff:ff:ff
>> # logout
>> $ ip link
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 3: em1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
>> state DOWN qlen 1000
>>
> I've probably misunderstood what you're trying to say. ifindexes are unique
> per
> boot and per netns.
I think we both misunderstood each other. The ifindexes are unique
*per netns*, which means that, if you're unprivileged in a netns,
global information doesn't leak to you. I think this is good.
>>
>> Let me try again, with emphasis in the right place.
>>
>> I think that *code running in a namespace* has no business even
>> knowing a unique identity of *that namespace* from the perspective of
>> the host.
>>
>> In your example, if there's a veth device between netns A and netns B,
>> then code *in netns A* has no business knowing the identity of its
>> veth peer if its peer (B) is a sibling or ancestor. It also IMO has
>> no business knowing the identity of its own netns (A) other than as
>> "my netns".
>
> I do not agree (see the example below).
>
>
>>
>> If A and B are siblings, then their parent needs to know where that
>> veth device goes, but I think this is already the case to a sufficient
>> extent today.
>
> I'm not aware of a hierarchy between netns. A daemon should be able to
> got the full network configuration, even if it's started when this
> configuration
> is already applied, ie even if it doesn't know what happen before it starts.
>
I don't know exactly which namespaces have an explicit hierarchy, but
there is certainly a hierarchy of *user* namespaces, and network
namespaces live in user namespaces, so they at least have somewhat of
a hierarchy.
>
>>
>> I feel like this discussion is falling into a common trap of new API
>> discussions. Can one of you who wants this API please articulate,
>> with a reasonably precise example, what it is that you want to do, why
>> you can't easily do it already, and how this API helps? I currently
>> understand how the API creates problems, but I don't understand how it
>> solves any problems, and I will NAK it (and I suspect that Eric will,
>> too, which is pretty much fatal) unless that changes.
>
> What I'm trying to solve is to have full info in netlink messages sent by
> the
> kernel, thus beeing able to identify a peer netns (and this is close from
> what
> audit guys are trying to have). Theorically, messages sent by the kernel can
> be
> reused as is to have the same configuration. This is not the case with
> x-netns
> devices. Here is an example, with ip tunnels:
>
> $ ip netns add 1
> $ ip link add ipip1 type ipip remote 10.16.0.121 local 10.16.0.249 dev eth0
> $ ip -d link ls ipip1
> 8: ipip1@eth0: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode
> DEFAULT group default
> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
> ipip remote 10.16.0.121 local 10.16.0.249 dev eth0 ttl inherit pmtudisc
> $ ip link set ipip1 netns 1
> $ ip netns exec 1 ip -d link ls ipip1
> 8: ipip1@tunl0: <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
> mode DEFAULT group default
> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
> ipip remote 10.16.0.121 local 10.16.0.249 dev tunl0 ttl inherit pmtudisc
>
> Now informations got with 'ip link' are wrong and incomplete:
> - the link dev is now tunl0 instead of eth0, because we only got an ifindex
> from the kernel without any netns informations.
> - the encapsulation addresses are not part of this netns but the user
> doesn't
> known that (still because netns info is missing). These IPv4 addresses
> may
> exist into this netns.
> - it's not possible to create the same netdevice with these infos.
>
Aha. That's a genuine problem.
Perhaps we need a concept of which netnses should be able to see each other.
I think I would be okay with a somewhat different outcome from your example:
$ ip netns exec 1 ip -d link ls ipip1
8: ipip1@[unknown device in another namespace]:
<POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
I think this outcome is mandatory if netns 1 lives in a subsidiary
user namespace.
Certainly, if you do the 'ip link' in the original namespace, I agree
that this should work.
For most namespace types, this all works transparently, since
everything has an real identity all the way up the hierarchy. Network
namespaces are different.
I don't think that exposing serial numbers in /proc is a good
solution, both for the reasons already described and because I don't
think that iproute2 should need to muck around with /proc to function
correctly. Eric, any clever ideas here? Do we need fancier netlink
messages for this?
--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* [PATCH net] xen-netback: move netif_napi_add before binding interrupt
From: Wei Liu @ 2014-08-25 15:44 UTC (permalink / raw)
To: xen-devel, netdev; +Cc: talex5, Wei Liu, Ian Campbell
Interrupt is enabled when bind_interdomain_evtchn_to_irqhandler returns.
If there's interrupt pending interrupt handler is invoked.
NAPI needs to be initialised before binding interrupt otherwise the
interrupt handler will try to scheduling a NAPI instance that is not
initialised yet, resulting in kernel OOPS.
This fixes a regression introduced in ea2c5e13 ("xen-netback: move NAPI
add/remove calls").
Ideally function calls to create kthreads should also be moved before
binding but I intent to fix this regression with minimal changes and
refactor the code with another patch.
Reported-by: Thomas Leonard <talex5@gmail.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/xen-netback/interface.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index e29e15d..f379689 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -576,6 +576,9 @@ int xenvif_connect(struct xenvif_queue *queue, unsigned long tx_ring_ref,
init_waitqueue_head(&queue->dealloc_wq);
atomic_set(&queue->inflight_packets, 0);
+ netif_napi_add(queue->vif->dev, &queue->napi, xenvif_poll,
+ XENVIF_NAPI_WEIGHT);
+
if (tx_evtchn == rx_evtchn) {
/* feature-split-event-channels == 0 */
err = bind_interdomain_evtchn_to_irqhandler(
@@ -629,9 +632,6 @@ int xenvif_connect(struct xenvif_queue *queue, unsigned long tx_ring_ref,
wake_up_process(queue->task);
wake_up_process(queue->dealloc_task);
- netif_napi_add(queue->vif->dev, &queue->napi, xenvif_poll,
- XENVIF_NAPI_WEIGHT);
-
return 0;
err_rx_unbind:
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH V4 3/8] namespaces: expose ns instance serial numbers in proc
From: Nicolas Dichtel @ 2014-08-25 15:43 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Serge E. Hallyn, Eric W. Biederman,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Linux API, Richard Guy Briggs,
netdev
In-Reply-To: <CALCETrW1Lv0qeccMjNHSEzgtiaNN3NgJVR1dFjjR_dw5KVVnqA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Le 25/08/2014 16:04, Andy Lutomirski a écrit :
> On Aug 25, 2014 6:30 AM, "Nicolas Dichtel" <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:
>>> CRIU wants to save the complete state of a namespace and then restore
>>> it. For that to work, any information exposed to things in the
>>> namespace *cannot* be globally unique or unique per boot, since CRIU
>>> needs to arrange for that information to match whatever it was when
>>> CRIU saved it.
>>
>> How are ifindex of network devices managed? These ifindexes are unique per boot,
>> thus can change depending on the order in which netdev are created.
>> These ifindexes are unique per boot and exposed to userspace ...
>>
>
> This does not appear to be true.
>
> $ sudo unshare --net
> # ip link add veth0 type veth peer name veth1
> # ip link
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 06:0d:59:c7:a6:a8 brd ff:ff:ff:ff:ff:ff
> 3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
> DEFAULT group default qlen 1000
> link/ether b2:5c:8b:f2:12:28 brd ff:ff:ff:ff:ff:ff
> # logout
> $ ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 3: em1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
> state DOWN qlen 1000
>
I've probably misunderstood what you're trying to say. ifindexes are unique per
boot and per netns. These ifindexes depend on the interface creation order:
$ ip netns add 1
$ ip link set eth1 netns 1
$ ip netns exec 1 ip link add veth0 type veth peer name veth1
$ ip netns exec 1 ip link
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT
group default qlen 1000
link/ether 9a:a0:89:99:a0:3c brd ff:ff:ff:ff:ff:ff
3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group
default qlen 1000
link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff
4: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT
group default qlen 1000
link/ether 96:86:44:49:ce:a8 brd ff:ff:ff:ff:ff:ff
$ ip netns del 1
$ ip netns add 1
$ ip netns exec 1 ip link add veth0 type veth peer name veth1
$ ip link set eth1 netns 1
$ ip netns exec 1 ip link
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT
group default qlen 1000
link/ether 86:92:90:01:32:6b brd ff:ff:ff:ff:ff:ff
3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT
group default qlen 1000
link/ether ae:8b:d2:71:48:a2 brd ff:ff:ff:ff:ff:ff
4: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group
default qlen 1000
link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff
Note: when an interface is moved to another netns, the ifindex is kept if
possible, else another ifindex is chosen.
I will dig a bit to understand how CRIU save these netns informations.
>>
>>>
>>> Also, I think that code running in a namespace has no business even
>>> knowing a unique identity of that namespace from the perspective of
>>> the host.
>>
>> Another scenario is when you have virtual network devices across two netns. You
>> need to identify the peer netns to have a netlink message which is fully interpretable by the userspace.
>
> Let me try again, with emphasis in the right place.
>
> I think that *code running in a namespace* has no business even
> knowing a unique identity of *that namespace* from the perspective of
> the host.
>
> In your example, if there's a veth device between netns A and netns B,
> then code *in netns A* has no business knowing the identity of its
> veth peer if its peer (B) is a sibling or ancestor. It also IMO has
> no business knowing the identity of its own netns (A) other than as
> "my netns".
I do not agree (see the example below).
>
> If A and B are siblings, then their parent needs to know where that
> veth device goes, but I think this is already the case to a sufficient
> extent today.
I'm not aware of a hierarchy between netns. A daemon should be able to
got the full network configuration, even if it's started when this configuration
is already applied, ie even if it doesn't know what happen before it starts.
>
> I feel like this discussion is falling into a common trap of new API
> discussions. Can one of you who wants this API please articulate,
> with a reasonably precise example, what it is that you want to do, why
> you can't easily do it already, and how this API helps? I currently
> understand how the API creates problems, but I don't understand how it
> solves any problems, and I will NAK it (and I suspect that Eric will,
> too, which is pretty much fatal) unless that changes.
What I'm trying to solve is to have full info in netlink messages sent by the
kernel, thus beeing able to identify a peer netns (and this is close from what
audit guys are trying to have). Theorically, messages sent by the kernel can be
reused as is to have the same configuration. This is not the case with x-netns
devices. Here is an example, with ip tunnels:
$ ip netns add 1
$ ip link add ipip1 type ipip remote 10.16.0.121 local 10.16.0.249 dev eth0
$ ip -d link ls ipip1
8: ipip1@eth0: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT
group default
link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
ipip remote 10.16.0.121 local 10.16.0.249 dev eth0 ttl inherit pmtudisc
$ ip link set ipip1 netns 1
$ ip netns exec 1 ip -d link ls ipip1
8: ipip1@tunl0: <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN mode
DEFAULT group default
link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
ipip remote 10.16.0.121 local 10.16.0.249 dev tunl0 ttl inherit pmtudisc
Now informations got with 'ip link' are wrong and incomplete:
- the link dev is now tunl0 instead of eth0, because we only got an ifindex
from the kernel without any netns informations.
- the encapsulation addresses are not part of this netns but the user doesn't
known that (still because netns info is missing). These IPv4 addresses may
exist into this netns.
- it's not possible to create the same netdevice with these infos.
Hope it's more clear now.
Regards,
Nicolas
^ permalink raw reply
* [PATCH v2 net-next] net: Functions to report space available in device TX queues
From: Tom Herbert @ 2014-08-25 15:27 UTC (permalink / raw)
To: davem, netdev
This patch adds netdev_tx_avail_queue and netdev_avail_queue which are
used to report number of bytes available in transmit queues per BQL. The
functions call dql_avail which returns BQL limit minus number of
inflight bytes. These functions can be called without txlock, for
instance to ascertain how much data should be dequeued from a qdisc in
a batch. When called without the tx_lock, the result is technically a
hint, subsequently when the tx_lock is done for a transmit it is
possible the availability has changed (for example a transmit
completion may have freed up more space in the queue or changed the
limit).
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/netdevice.h | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0fac884..bdf6c85 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2544,6 +2544,30 @@ static inline void netdev_completed_queue(struct net_device *dev,
netdev_tx_completed_queue(netdev_get_tx_queue(dev, 0), pkts, bytes);
}
+static inline int netdev_tx_avail_queue(struct netdev_queue *dev_queue)
+{
+#ifdef CONFIG_BQL
+ return dql_avail(&dev_queue->dql);
+#else
+ return DQL_MAX_LIMIT;
+#endif
+}
+
+/**
+ * netdev_avail_queue - report how much space is availble for xmit
+ * @dev: network device
+ *
+ * Report the amount of space available in the TX queue in terms of
+ * number of bytes. This returns the number of bytes avaiable per
+ * DQL. This function may be called without taking the txlock on
+ * the device, however in that case the result should be taken as
+ * a (strong) hint.
+ */
+static inline int netdev_avail_queue(struct net_device *dev)
+{
+ return netdev_tx_avail_queue(netdev_get_tx_queue(dev, 0));
+}
+
static inline void netdev_tx_reset_queue(struct netdev_queue *q)
{
#ifdef CONFIG_BQL
@@ -2559,9 +2583,9 @@ static inline void netdev_tx_reset_queue(struct netdev_queue *q)
* Reset the bytes and packet count of a network device and clear the
* software flow control OFF bit for this network device
*/
-static inline void netdev_reset_queue(struct net_device *dev_queue)
+static inline void netdev_reset_queue(struct net_device *dev)
{
- netdev_tx_reset_queue(netdev_get_tx_queue(dev_queue, 0));
+ netdev_tx_reset_queue(netdev_get_tx_queue(dev, 0));
}
/**
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH net-next] net: Functions to report space available in device TX queues
From: Tom Herbert @ 2014-08-25 15:18 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <20140824.223529.704627469513160252.davem@davemloft.net>
On Sun, Aug 24, 2014 at 10:35 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Sun, 24 Aug 2014 13:19:47 -0700 (PDT)
>
>> +/**
>> + * netdev_avail_queue - report how much space is availble for xmit
>> + * @dev: network device
>> + *
>> + * Report the amount of space available in the TX queue in terms of
>> + * number of bytes. This returns the number of bytes avaiable per
>> + * DQL. This function may be called without taking the txlock on
>> + * the device, however in that case the result should be taken as
>> + * a (strong) hint.
>> + */
>> +static inline int netdev_avail_queue(struct net_device *dev_queue)
>> +{
>> + return netdev_tx_avail_queue(netdev_get_tx_queue(dev_queue, 0));
>> +}
>> +
>
> This doesn't make any sense, you're only providing queue zero's
> information.
>
It's consistent with the other BQL functions. netdev_*_queue is used
for single queue devices, netdev_tx_*_queue is used for specific
queues in MQ.
> You're passing in a net_device, calling it a "dev_queue" in the
> variable name, the exlicitly using queue zero of that device in the
> netdev_get_tx_queue() call.
>
Cut and paste prototype from netdev_reset_queue which was doing
that... I'll fix it.
> Pretty confusing if you ask me :)
^ permalink raw reply
* Re: [RFC PATCH net-next 3/3] packet: make use of deferred TX queue flushing
From: Jesper Dangaard Brouer @ 2014-08-25 15:16 UTC (permalink / raw)
Cc: brouer, Daniel Borkmann, davem, netdev
In-Reply-To: <20140825155402.2f2a03d7@redhat.com>
On Mon, 25 Aug 2014 15:54:02 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Sun, 24 Aug 2014 15:42:18 +0200
> Daniel Borkmann <dborkman@redhat.com> wrote:
>
> > This adds a first use-case of deferred tail pointer flushing
> > for AF_PACKET's TX_RING in QDISC_BYPASS mode.
>
> Testing with trafgen. I've updated patch 1/3 to NOT call mmiowb(),
> during this testing, see why in my other post.
>
> trafgen cmdline:
> trafgen --cpp --dev eth5 --conf udp_example01.trafgen -V --cpus 1
> * Only use 1 CPU
> * default is mmap
> * default is QDISC_BYPASS mode
>
> BASELINE(no-patches): trafgen QDISC_BYPASS and mmap:
> - tx:1562539 pps
>
> With PACKET_FLUSH_THRESH=8, and QDISC_BYPASS and mmap:
> - tx:1683746 pps
>
> Improvement:
> + 121207 pps
> - 46 ns (1/1562539*10^9)-(1/1683746*10^9)
>
> This is a significant improvement! :-)
I'm unfortunately seeing a regression, if I'm NOT bypassing the qdisc
layer, and still use mmap. Trafgen have an option --qdisc-path for
this. (I believe most other solutions, don't set the QDISC_BYPASS
socket option)
trafgen command:
# trafgen --cpp --dev eth5 --conf udp_example01.trafgen -V --qdisc-path --cpus 1
* still use mmap
* choose normal qdisc code path via --qdisc-path
BASELINE(no-patches): trafgen using --qdisc-path and mmap:
- tx:1371307 pps
(Patched): trafgen using --qdisc-path and mmap
- tx:1345999 pps
Regression:
* 25308 pps slower than before
* 13.71 nanosec slower (1/1371307*10^9)-(1/1345999*10^9)
How can we explain this?!?
As can be deducted from the baseline numbers, the cost of the qdisc
path is fairly high, with 89.24 ns ((1/1562539*10^9)-(1/1371307*10^9)).
(This is a bit higher than I expected based on my data from:
http://people.netfilter.org/hawk/presentations/nfws2014/dp-accel-qdisc-lockless.pdf
where I measured it to be 60ns).
(Does this makes sense?): Above results say we can save 46ns by
delaying tailptr updates. But the qdisc path itself will add 89ns of
delay between packet, which is then too large to take advantage of the
tailptr win. (not sure this explains the issue... feel free to come up
with a better explanation)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net: stmmac: add dcrs parameter
From: Vince Bridgers @ 2014-08-25 15:10 UTC (permalink / raw)
To: Giuseppe CAVALLARO
Cc: Chen-Yu Tsai, Ley Foon Tan, netdev, linux-kernel, David S. Miller,
lftan.linux, Vince Bridgers
In-Reply-To: <53FB313C.9010808@st.com>
Hi,
On Mon, Aug 25, 2014 at 7:51 AM, Giuseppe CAVALLARO
<peppe.cavallaro@st.com> wrote:
> On 8/25/2014 2:34 PM, Chen-Yu Tsai wrote:
>>
>> Hi,
>>
>> On Mon, Aug 25, 2014 at 7:50 PM, Ley Foon Tan <lftan@altera.com> wrote:
>>>
>>> This patch add the option to enable DCRS bit in GMAC control register.
>>> Default is disabled if snps,dcrs is not defined.
>>>
>>> For MII, Carrier Sense (CRS) must be asserted during transmission
>>> whereas in RGMII, CRS is not. RGMII does not provide a way to signal
>>> loss of carrier during a transmission.
While technically true, from a practical point of view, this is only
useful if using true half-duplex media (like the now obsolete 10Base2
and 10Base5 - think old school coax with vampire taps).
>>>
>>> When DCRS bit set high in control register, the MAC transmitter
>>> ignore the (G)MII Carrier Sense signal during frame transmission
>>> in the half-duplex mode. This request results in no errors generated
>>> because of Loss of Carrier or No Carrier during such transmission.
>>>
>>> Signed-off-by: Ley Foon Tan <lftan@altera.com>
>>> ---
<snip>
>>
>> Since you know this is only required under (G)MII, could you not re-use
>> the "phy-mode" property, instead of adding another one?
>>
>> Better yet, use the "interface" field in the platform data. This way
>> you'll
>> fix non-DT devices as well. You could then avoid touching the platform
>> driver,
>> and just modify the driver core.
>
>
> yes this is what I asked. Thx ChenYu for the this detail.
> Ley Foon Tan, could you do that? Let me know
>
> peppe
>
>
In the Synopsys EMAC case, carrier sense is used to stop transmitting
if no carrier is sensed during a transmission. This is only useful if
the media in use is true half duplex media (like obsolete 10Base2 or
10Base5). If no one in using true half duplex media, then is it
possible to set this disable by default? If we're not sure, then
having an option feels like the right thing to do.
Vince
^ permalink raw reply
* Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
From: Richard Weinberger @ 2014-08-25 15:07 UTC (permalink / raw)
To: KY Srinivasan, Sitsofe Wheeler
Cc: Greg Kroah-Hartman, Jason Wang, linux-kernel@vger.kernel.org,
David S. Miller, Daniel Borkmann, netdev@vger.kernel.org,
devel@linuxdriverproject.org, Haiyang Zhang
In-Reply-To: <2c4d3b36d8be4fbf884d78c7507a29af@BY2PR0301MB0711.namprd03.prod.outlook.com>
Am 25.08.2014 16:53, schrieb KY Srinivasan:
>
>
>> -----Original Message-----
>> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
>> Sent: Monday, August 25, 2014 3:48 AM
>> To: Sitsofe Wheeler
>> Cc: Haiyang Zhang; KY Srinivasan; Greg Kroah-Hartman;
>> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; Jason Wang;
>> Daniel Borkmann; David S. Miller; netdev@vger.kernel.org
>> Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
>>
>> via
>>
>>
>> On Wed, Aug 20, 2014 at 5:41 AM, Sitsofe Wheeler <sitsofe@gmail.com>
>> wrote:
>>> Aug 20 04:04:41 ubuntuhv kernel: [ 9.230399] random: nonblocking pool is
>> initialized
>>> Aug 20 04:04:41 ubuntuhv kernel: [ 10.338487] EXT4-fs (sda1): re-
>> mounted. Opts: errors=remount-ro
>>> Aug 20 04:04:41 ubuntuhv kernel: [ 11.099094] hv_storvsc vmbus_0_1:
>> cmd 0x85 scsi status 0x2 srb status 0x6
>>> Aug 20 04:04:41 ubuntuhv kernel: [ 11.099901] hv_storvsc vmbus_0_1:
>> cmd 0x85 scsi status 0x2 srb status 0x6
>>> Aug 20 04:04:43 ubuntuhv kernel: [ 12.999830] psmouse serio1: trackpoint:
>> IBM TrackPoint firmware: 0x01, buttons: 0/0
>>> Aug 20 03:55:47 ubuntuhv kernel: [ 13.003659] input: TPPS/2 IBM
>> TrackPoint as /devices/platform/i8042/serio1/input/input4
>>> Aug 20 03:57:28 ubuntuhv kernel: [ 113.711832] hv_netvsc vmbus_0_14:
>>> net device safe to remove Aug 20 03:57:28 ubuntuhv kernel: [
>>> 113.713882] hv_netvsc: hv_netvsc channel opened successfully Aug 20
>>> 03:57:29 ubuntuhv kernel: [ 114.961312] hv_netvsc vmbus_0_14: Send
>>> section size: 6144, Section count:2560 Aug 20 03:57:29 ubuntuhv
>>> kernel: [ 114.962711] hv_netvsc vmbus_0_14: Device MAC
>>> 00:15:5d:6f:02:af link state up Aug 20 03:57:34 ubuntuhv kernel: [
>>> 120.027718] hv_netvsc vmbus_0_14: net device safe to remove Aug 20
>>> 03:57:34 ubuntuhv kernel: [ 120.030047] hv_netvsc: hv_netvsc channel
>>> opened successfully Aug 20 03:57:34 ubuntuhv kernel: [ 120.035422]
>> hv_netvsc vmbus_0_14 eth0: unable to establish receive buffer's gpadl Aug
>> 20 03:57:34 ubuntuhv kernel: [ 120.039778] hv_netvsc vmbus_0_14 eth0:
>> unable to connect to NetVSP - 4 Aug 20 03:57:34 ubuntuhv kernel: [
>> 120.039818] ------------[ cut here ]------------ Aug 20 03:57:34 ubuntuhv kernel:
>> [ 120.039832] kernel BUG at drivers/hv/channel.c:504!
>>
>> This is one is also a rude BUG_ON:
>> ret = vmbus_post_msg(msg, sizeof(struct
>> vmbus_channel_close_channel));
>>
>> BUG_ON(ret != 0);
>>
>> vmbus_post_msg() hv_post_message() can easily return !0.
>> i.e. if this kmalloc() fails:
>> addr = (unsigned long)kmalloc(sizeof(struct aligned_input),
>> GFP_ATOMIC);
>> if (!addr)
>> return -ENOMEM;
>
> I will submit a patch to handle this case. I suspect though that the original ASSERT at channel.c (line 504)
> is not related to kmalloc failing in vmbus_post_msg(). I will also look at that issue as well.
I'm sure you can replace most BUG_ON() by a WARN_ON().
Also print more details why the assertion does not hold.
Thanks,
//richard
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Thomas Graf @ 2014-08-25 14:54 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Scott Feldman, John Fastabend, Jiri Pirko, netdev, davem, nhorman,
andy, dborkman, ogerlitz, jesse, pshelar, azhou, ben, stephen,
jeffrey.t.kirsher, vyasevic, xiyou.wangcong, john.r.fastabend,
edumazet, f.fainelli, roopa, linville, dev, jasowang, ebiederm,
nicolas.dichtel, ryazanov.s.a, buytenh, aviadr, nbd,
alexei.starovoitov, Neil.Jerram, ronye
In-Reply-To: <53FA01AC.10507@mojatatu.com>
On 08/24/14 at 11:15am, Jamal Hadi Salim wrote:
> The focus of the patches is on offloading flows (uses the
> ovs or shall i say the broadcom OF-DPA API, which is one
> vendor's view of the world).
Let's keep vendors out of this discussion. I have no affiliation
with this vendor. In fact I'm personally more interested in the
host use case with the biggest concerns/focus on integration with
existing APIs.
> >It proposes *a* interface which in this case is flow based with mask
> >support to accomodate the typical ntuple filter API in HW. OVS happens
> >to be one of the easiest to use examples as a consumer because it
> >already provides a flat flow representation.
> >
>
> In other words, there is a direct 1-1 map between this approach and OVS.
> That is a contentious point.
That is simply not the case. The fact that John is using this model
to replace the flow director ioctl API should prove this.
> Not at all.
> I gave an example earlier with u32, but lets pick the other extreme
> of well understood functions, say L3 (I could pick L2 as well).
> This openflow api tries to describe different header
There is not a single bit specific to OpenFlow and there is absolutely
no awareness of OF within the kernel in OVS.
> fields in the packet. That is not the challenge for such an
> API. The challenge is dealing with the quarks.
> Some chips implement FIB and NH conjoined; others implement
> them separately.
> I dont see how this is even being remotely touched on.
First of all, that sounds like exactly like something that should
be handled in the driver specific portion of the API. Secondly,
can you provide additional information on these specific pieces of
hardware so we take it into account?
> You are asking me to go and add a new ndo() every time i have a new network
> function? That is not scalable. I have no problem with
> the approach that was posted - I have a problem that it is it
> focused on flows (and is lacking ability to specify different
> classifiers). It should not be called xxx_flow_xxx
Realistically there will only be a handful, maybe something
like:
flow_insert / flow_remove
p4_add / p4_remove
[...]
Maybe you can share some information the specific API you have
in mind?
> If you looked at all my presentations I have never laid such
> claim but i have always said I want everything described in
> iproute2 to work. I dont think anyone disagreed.
> I dont expect tc to be used as *the interface*; but on the same
> token i dont expect OVS to be used as *the interface*.
Agreed, I don't think anybody expects anything else.
> Lets start with hardware abstraction. Lets map to existing Linux APIs
> and then see where some massaging maybe needed.
That's what's being done. HW offload is being mapped to OVS and
to an existing ioctl interface. Those are existing Linux APIs.
Can you explain why swdev as proposed is not suitable for the
other existing Linux APIs? They don't *have* to use the flow_insert(),
they are free to exted the API to represent more generic programmable
hardware.
> This abstraction gives OVS 1-1 mapping which is something i object to.
> You want to penalize me for the sake of getting the OVS api in place?
I don't understand this.
> Beginning with flows and laying claim to that one would be able to
> cover everything is non-starter.
Nobody claims that. In fact, I'm very interested in seeing the API
extended for non flow based models. I'm actually convinced that flow
based models are not the ultimate answers on HW level but a vast majority
of hardware understands some form of protocol aware exact match or
wildcard filters of limited capacity. This category of hardware is
being addressed with the flow_insert() API.
> I will simplify:
> You cant possibly do the u32 classifier completely using the posted
> hard-coded 15 tuple classifier. It is an NP-complete problem.
> There are *a lot* of use cases which can be specified by u32 that are
> not possible to specify with the tuples the patches posted propose.
> The reverse is not true. You can fully specify the OVS classifier
> with u32.
> So if you want to specify the closest to a universal grammar for
> specifying a classifier - use u32 and create templates for your
> classifier.
Completely agreed, this is why we have cls/act and nftables.
> There are some cases where that approach doesnt make sense:
> example if i wanted to specify a string classifier etc.
> But if we are talking packet header classifier - it is flexible.
> There are also good reasons to specify a universal 5 tuple classifier.
> As there are good reasons to specify your latest OF classifier.
> But that OF classifier being the starting point is not pragmatic.
So you agree that at least on the driver level some form of ntuple
awareness must be given because the hardware has limited capabilities.
This is exactly what flow_insert() is, it is a generic ntuple
classifier which can implement a subset of the 15 tuple in HW. So
instead of adding a separate NDO for each fixed tuple, a generic
NDO can handle the different levels of offloads. Very similar to how
the xmit to the NIC can handle various protocol offloads already.
What is being proposed is a generic ntuple with masking support to
describe filtering needs. What is missing is a capabilities reporting
channel so API users can know in advance what is supported to
implement partial offloads.
^ permalink raw reply
* RE: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
From: KY Srinivasan @ 2014-08-25 14:53 UTC (permalink / raw)
To: Richard Weinberger, Sitsofe Wheeler
Cc: Greg Kroah-Hartman, Jason Wang, linux-kernel@vger.kernel.org,
David S. Miller, Daniel Borkmann, netdev@vger.kernel.org,
devel@linuxdriverproject.org, Haiyang Zhang
In-Reply-To: <CAFLxGvx5_JSb7sRPXY01d+eNJ4p4fk=MySGyui91KNbdyLyQ_g@mail.gmail.com>
> -----Original Message-----
> From: Richard Weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Monday, August 25, 2014 3:48 AM
> To: Sitsofe Wheeler
> Cc: Haiyang Zhang; KY Srinivasan; Greg Kroah-Hartman;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; Jason Wang;
> Daniel Borkmann; David S. Miller; netdev@vger.kernel.org
> Subject: Re: [hyperv] BUG at drivers/hv/channel.c:462 while changing MTU
>
> via
>
>
> On Wed, Aug 20, 2014 at 5:41 AM, Sitsofe Wheeler <sitsofe@gmail.com>
> wrote:
> > Aug 20 04:04:41 ubuntuhv kernel: [ 9.230399] random: nonblocking pool is
> initialized
> > Aug 20 04:04:41 ubuntuhv kernel: [ 10.338487] EXT4-fs (sda1): re-
> mounted. Opts: errors=remount-ro
> > Aug 20 04:04:41 ubuntuhv kernel: [ 11.099094] hv_storvsc vmbus_0_1:
> cmd 0x85 scsi status 0x2 srb status 0x6
> > Aug 20 04:04:41 ubuntuhv kernel: [ 11.099901] hv_storvsc vmbus_0_1:
> cmd 0x85 scsi status 0x2 srb status 0x6
> > Aug 20 04:04:43 ubuntuhv kernel: [ 12.999830] psmouse serio1: trackpoint:
> IBM TrackPoint firmware: 0x01, buttons: 0/0
> > Aug 20 03:55:47 ubuntuhv kernel: [ 13.003659] input: TPPS/2 IBM
> TrackPoint as /devices/platform/i8042/serio1/input/input4
> > Aug 20 03:57:28 ubuntuhv kernel: [ 113.711832] hv_netvsc vmbus_0_14:
> > net device safe to remove Aug 20 03:57:28 ubuntuhv kernel: [
> > 113.713882] hv_netvsc: hv_netvsc channel opened successfully Aug 20
> > 03:57:29 ubuntuhv kernel: [ 114.961312] hv_netvsc vmbus_0_14: Send
> > section size: 6144, Section count:2560 Aug 20 03:57:29 ubuntuhv
> > kernel: [ 114.962711] hv_netvsc vmbus_0_14: Device MAC
> > 00:15:5d:6f:02:af link state up Aug 20 03:57:34 ubuntuhv kernel: [
> > 120.027718] hv_netvsc vmbus_0_14: net device safe to remove Aug 20
> > 03:57:34 ubuntuhv kernel: [ 120.030047] hv_netvsc: hv_netvsc channel
> > opened successfully Aug 20 03:57:34 ubuntuhv kernel: [ 120.035422]
> hv_netvsc vmbus_0_14 eth0: unable to establish receive buffer's gpadl Aug
> 20 03:57:34 ubuntuhv kernel: [ 120.039778] hv_netvsc vmbus_0_14 eth0:
> unable to connect to NetVSP - 4 Aug 20 03:57:34 ubuntuhv kernel: [
> 120.039818] ------------[ cut here ]------------ Aug 20 03:57:34 ubuntuhv kernel:
> [ 120.039832] kernel BUG at drivers/hv/channel.c:504!
>
> This is one is also a rude BUG_ON:
> ret = vmbus_post_msg(msg, sizeof(struct
> vmbus_channel_close_channel));
>
> BUG_ON(ret != 0);
>
> vmbus_post_msg() hv_post_message() can easily return !0.
> i.e. if this kmalloc() fails:
> addr = (unsigned long)kmalloc(sizeof(struct aligned_input),
> GFP_ATOMIC);
> if (!addr)
> return -ENOMEM;
I will submit a patch to handle this case. I suspect though that the original ASSERT at channel.c (line 504)
is not related to kmalloc failing in vmbus_post_msg(). I will also look at that issue as well.
Regards,
K. Y
>
> --
> Thanks,
> //richard
^ permalink raw reply
* RE: [PATCH v2 8/8] qlge: Fix TSO for non-accelerated vlan traffic
From: Shahed Shaikh @ 2014-08-25 14:52 UTC (permalink / raw)
To: Vladislav Yasevich, netdev
Cc: Vladislav Yasevich, Ron Mercer, Dept-Eng Linux Driver,
Dept-GE Linux NIC Dev
In-Reply-To: <1408977295-9162-9-git-send-email-vyasevic@redhat.com>
> -----Original Message-----
> From: Vladislav Yasevich [mailto:vyasevich@gmail.com]
> Sent: Monday, August 25, 2014 8:05 PM
> To: netdev
> Cc: Vladislav Yasevich; Shahed Shaikh; zz-930533; Ron Mercer; Dept-Eng Linux
> Driver
> Subject: [PATCH v2 8/8] qlge: Fix TSO for non-accelerated vlan traffic
>
> This device claims TSO support for vlans. It also allows a user to control vlan
> acceleration offloading. As such, it is possible to turn off vlan acceleration
> and configure a vlan which will continue to send TSO traffic.
>
> In such situation the packet passed down the the device will contain a vlan
> header and skb->protocol will be set to ETH_P_8021Q.
> The device assumes that skb->protocol contains network protocol value and
> uses that value to set up TSO information.
> This results in corrupted frames sent on the wire.
>
> This patch extracts the protocol value correctly by using a
> vlan_get_protocol() helper and corrects corrupt TSO frames.
>
> CC: Shahed Shaikh <shahed.shaikh@qlogic.com>
> CC: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> CC: Ron Mercer <ron.mercer@qlogic.com>
> CC: linux-driver@qlogic.com
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
Thanks,
Shahed
> ---
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 188626e..3e96f26 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -2556,6 +2556,7 @@ static int ql_tso(struct sk_buff *skb, struct
> ob_mac_tso_iocb_req *mac_iocb_ptr)
>
> if (skb_is_gso(skb)) {
> int err;
> + __be16 l3_proto = vlan_get_protocol(skb);
>
> err = skb_cow_head(skb, 0);
> if (err < 0)
> @@ -2572,7 +2573,7 @@ static int ql_tso(struct sk_buff *skb, struct
> ob_mac_tso_iocb_req *mac_iocb_ptr)
> << OB_MAC_TRANSPORT_HDR_SHIFT);
> mac_iocb_ptr->mss = cpu_to_le16(skb_shinfo(skb)-
> >gso_size);
> mac_iocb_ptr->flags2 |= OB_MAC_TSO_IOCB_LSO;
> - if (likely(skb->protocol == htons(ETH_P_IP))) {
> + if (likely(l3_proto == htons(ETH_P_IP))) {
> struct iphdr *iph = ip_hdr(skb);
> iph->check = 0;
> mac_iocb_ptr->flags1 |= OB_MAC_TSO_IOCB_IP4;
> @@ -2580,7 +2581,7 @@ static int ql_tso(struct sk_buff *skb, struct
> ob_mac_tso_iocb_req *mac_iocb_ptr)
> iph->daddr,
> 0,
>
> IPPROTO_TCP,
> 0);
> - } else if (skb->protocol == htons(ETH_P_IPV6)) {
> + } else if (l3_proto == htons(ETH_P_IPV6)) {
> mac_iocb_ptr->flags1 |= OB_MAC_TSO_IOCB_IP6;
> tcp_hdr(skb)->check =
> ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> --
> 1.9.3
________________________________
This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
^ permalink raw reply
* Re: [PATCH net-next v3 09/12] net: dsa: add Broadcom tag RX/TX handler
From: Alexander Duyck @ 2014-08-25 14:51 UTC (permalink / raw)
To: Florian Fainelli, Alexander Duyck, netdev; +Cc: davem, jhs, linville
In-Reply-To: <53FAA158.6050600@gmail.com>
On 08/24/2014 07:37 PM, Florian Fainelli wrote:
> Le 24/08/2014 15:51, Alexander Duyck a écrit :
>> On 08/24/2014 11:44 AM, Florian Fainelli wrote:
>>> +
>>> +static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
>>> + struct packet_type *pt, struct net_device *orig_dev)
>>> +{
>>> + struct dsa_switch_tree *dst = dev->dsa_ptr;
>>> + struct dsa_switch *ds;
>>> + int source_port;
>>> + u8 *brcm_tag;
>>> +
>>> + if (unlikely(dst == NULL))
>>> + goto out_drop;
>>> +
>>> + ds = dst->ds[0];
>>> +
>>> + skb = skb_unshare(skb, GFP_ATOMIC);
>>> + if (skb == NULL)
>>> + goto out;
>>
>> At this point here we already have the dsa pointers and could just pull
>> up a function pointer so all of the code below could potentially be
>> moved into a separate function allowing us to drop the need to have
>> multiple Ethertypes and so we could just use ETH_P_DSA for all DSA
>> tagging type.
>
> I see, or rather maybe just use ETH_P_EDSA which is a real assigned
> ethertype?
> --
> Florian
I suppose we could probably do that. Although I would probably say we
should just rename it to ETH_P_DSA since we can just go through and drop
all of the other defined Ethertypes for DSA. In addition since the EDSA
isn't actually a registered type we probably don't need to bother with
conserving the name anyway.
Thanks,
Alex
^ permalink raw reply
* [PATCH net-next 2/2] tipc: add name distributor resiliency queue
From: erik.hugne @ 2014-08-25 14:54 UTC (permalink / raw)
To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne
In-Reply-To: <1408978469-10584-1-git-send-email-erik.hugne@ericsson.com>
From: Erik Hugne <erik.hugne@ericsson.com>
TIPC name table updates are distributed asynchronously in a cluster,
entailing a risk of certain race conditions. E.g., if two nodes
simultaneously issue conflicting (overlapping) publications, this may
not be detected until both publications have reached a third node, in
which case one of the publications will be silently dropped on that
node. Hence, we end up with an inconsistent name table.
In most cases this conflict is just a temporary race, e.g., one
node is issuing a publication under the assumption that a previous,
conflicting, publication has already been withdrawn by the other node.
However, because of the (rtt related) distributed update delay, this
may not yet hold true on all nodes. The symptom of this failure is a
syslog message: "tipc: Cannot publish {%u,%u,%u}, overlap error".
In this commit we add a resiliency queue at the receiving end of
the name table distributor. When insertion of an arriving publication
fails, we retain it in this queue for a short amount of time, assuming
that another update will arrive very soon and clear the conflict. If so
happens, we insert the publication, otherwise we drop it.
The (configurable) retention value defaults to 2000 ms. Knowing from
experience that the situation described above is extremely rare, there
is no risk that the queue will accumulate any large number of items.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
Documentation/sysctl/net.txt | 14 +++++++++
net/tipc/core.h | 1 +
net/tipc/name_distr.c | 69 ++++++++++++++++++++++++++++++++++++++++++--
net/tipc/name_distr.h | 1 +
net/tipc/name_table.c | 8 ++---
net/tipc/sysctl.c | 7 +++++
6 files changed, 93 insertions(+), 7 deletions(-)
diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 9a0319a..89ce54e 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -241,6 +241,9 @@ address of the router (or Connected) for internal networks.
6. TIPC
-------------------------------------------------------
+tipc_rmem
+----------
+
The TIPC protocol now has a tunable for the receive memory, similar to the
tcp_rmem - i.e. a vector of 3 INTEGERs: (min, default, max)
@@ -252,3 +255,14 @@ The max value is set to CONN_OVERLOAD_LIMIT, and the default and min values
are scaled (shifted) versions of that same value. Note that the min value
is not at this point in time used in any meaningful way, but the triplet is
preserved in order to be consistent with things like tcp_rmem.
+
+named_timeout
+--------------
+
+TIPC name table updates are distributed asynchronous in a cluster and there is
+no guarantees that a TIPC name withdrawal sent by a node have been received by
+any other given node before another name that would be considered an illegal
+overlap are received from another node.
+If named_timeout is nonzero, failed topology updates will be placed on a defer
+queue until another event arrives that clears the error, or until the timeout
+expires. Value is in milliseconds.
diff --git a/net/tipc/core.h b/net/tipc/core.h
index d2607a8..f773b14 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -81,6 +81,7 @@ extern u32 tipc_own_addr __read_mostly;
extern int tipc_max_ports __read_mostly;
extern int tipc_net_id __read_mostly;
extern int sysctl_tipc_rmem[3] __read_mostly;
+extern int sysctl_tipc_named_timeout __read_mostly;
/*
* Other global variables
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 0591f33..0cbe5e1 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -1,7 +1,7 @@
/*
* net/tipc/name_distr.c: TIPC name distribution code
*
- * Copyright (c) 2000-2006, Ericsson AB
+ * Copyright (c) 2000-2006, 2014, Ericsson AB
* Copyright (c) 2005, 2010-2011, Wind River Systems
* All rights reserved.
*
@@ -71,6 +71,21 @@ static struct publ_list *publ_lists[] = {
};
+int sysctl_tipc_named_timeout __read_mostly = 2000;
+
+/**
+ * struct tipc_dist_queue - queue holding deferred name table updates
+ */
+static struct list_head tipc_dist_queue = LIST_HEAD_INIT(tipc_dist_queue);
+
+struct distr_queue_item {
+ struct distr_item i;
+ u32 dtype;
+ u32 node;
+ u64 expiry;
+ struct list_head next;
+};
+
/**
* publ_to_item - add publication info to a publication message
*/
@@ -299,6 +314,52 @@ struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
}
/**
+ * tipc_named_add_backlog - add a failed name table update to the backlog
+ *
+ */
+static void tipc_named_add_backlog(struct distr_item *i, u32 type, u32 node)
+{
+ struct distr_queue_item *e;
+ u64 now = get_jiffies_64();
+
+ e = kzalloc(sizeof(*e), GFP_ATOMIC);
+ if (!e)
+ return;
+ e->dtype = type;
+ e->node = node;
+ e->expiry = now + msecs_to_jiffies(sysctl_tipc_named_timeout);
+ memcpy(e, i, sizeof(*i));
+ list_add_tail(&e->next, &tipc_dist_queue);
+}
+
+/**
+ * tipc_named_process_backlog - try to process any pending name table updates
+ * from the network.
+ */
+void tipc_named_process_backlog(void)
+{
+ struct distr_queue_item *e, *tmp;
+ char addr[16];
+ u64 now = get_jiffies_64();
+
+ list_for_each_entry_safe(e, tmp, &tipc_dist_queue, next) {
+ if (e->expiry > now) {
+ if (!tipc_update_nametbl(&e->i, e->node, e->dtype))
+ continue;
+ } else {
+ tipc_addr_string_fill(addr, e->node);
+ pr_warn_ratelimited("Dropping name table update (%d) of {%u, %u, %u} from %s key=%u\n",
+ e->dtype, ntohl(e->i.type),
+ ntohl(e->i.lower),
+ ntohl(e->i.upper),
+ addr, ntohl(e->i.key));
+ }
+ list_del(&e->next);
+ kfree(e);
+ }
+}
+
+/**
* tipc_named_rcv - process name table update message sent by another node
*/
void tipc_named_rcv(struct sk_buff *buf)
@@ -306,13 +367,15 @@ void tipc_named_rcv(struct sk_buff *buf)
struct tipc_msg *msg = buf_msg(buf);
struct distr_item *item = (struct distr_item *)msg_data(msg);
u32 count = msg_data_sz(msg) / ITEM_SIZE;
+ u32 node = msg_orignode(msg);
write_lock_bh(&tipc_nametbl_lock);
while (count--) {
- tipc_update_nametbl(item, msg_orignode(msg),
- msg_type(msg));
+ if (!tipc_update_nametbl(item, node, msg_type(msg)))
+ tipc_named_add_backlog(item, msg_type(msg), node);
item++;
}
+ tipc_named_process_backlog();
write_unlock_bh(&tipc_nametbl_lock);
kfree_skb(buf);
}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index 8afe32b..b9e75fe 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -73,5 +73,6 @@ void named_cluster_distribute(struct sk_buff *buf);
void tipc_named_node_up(u32 dnode);
void tipc_named_rcv(struct sk_buff *buf);
void tipc_named_reinit(void);
+void tipc_named_process_backlog(void);
#endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c058e30..3a6a0a7 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -261,8 +261,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
/* Lower end overlaps existing entry => need an exact match */
if ((sseq->lower != lower) || (sseq->upper != upper)) {
- pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
- type, lower, upper);
return NULL;
}
@@ -284,8 +282,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
/* Fail if upper end overlaps into an existing entry */
if ((inspos < nseq->first_free) &&
(upper >= nseq->sseqs[inspos].lower)) {
- pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
- type, lower, upper);
return NULL;
}
@@ -677,6 +673,8 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
if (likely(publ)) {
table.local_publ_count++;
buf = tipc_named_publish(publ);
+ /* Any pending external events? */
+ tipc_named_process_backlog();
}
write_unlock_bh(&tipc_nametbl_lock);
@@ -698,6 +696,8 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
if (likely(publ)) {
table.local_publ_count--;
buf = tipc_named_withdraw(publ);
+ /* Any pending external events? */
+ tipc_named_process_backlog();
write_unlock_bh(&tipc_nametbl_lock);
list_del_init(&publ->pport_list);
kfree(publ);
diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c
index f3fef93..1a779b1 100644
--- a/net/tipc/sysctl.c
+++ b/net/tipc/sysctl.c
@@ -47,6 +47,13 @@ static struct ctl_table tipc_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "named_timeout",
+ .data = &sysctl_tipc_named_timeout,
+ .maxlen = sizeof(sysctl_tipc_named_timeout),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{}
};
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 1/2] tipc: refactor name table updates out of named packet receive routine
From: erik.hugne @ 2014-08-25 14:54 UTC (permalink / raw)
To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne
From: Erik Hugne <erik.hugne@ericsson.com>
We need to perform the same actions when processing deferred name
table updates, so this functionality is moved to a separate
function.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_distr.c | 74 ++++++++++++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 36 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index dcc15bc..0591f33 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -263,52 +263,54 @@ static void named_purge_publ(struct publication *publ)
}
/**
+ * tipc_update_nametbl - try to process a nametable update and notify
+ * subscribers
+ *
+ * tipc_nametbl_lock must be held.
+ * Returns the publication item if successful, otherwise NULL.
+ */
+struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
+ u32 dtype)
+{
+ struct publication *publ = NULL;
+
+ if (dtype == PUBLICATION) {
+ publ = tipc_nametbl_insert_publ(ntohl(i->type), ntohl(i->lower),
+ ntohl(i->upper),
+ TIPC_CLUSTER_SCOPE, node,
+ ntohl(i->ref), ntohl(i->key));
+ if (publ) {
+ tipc_nodesub_subscribe(&publ->subscr, node, publ,
+ (net_ev_handler)
+ named_purge_publ);
+ }
+ } else if (dtype == WITHDRAWAL) {
+ publ = tipc_nametbl_remove_publ(ntohl(i->type), ntohl(i->lower),
+ node, ntohl(i->ref),
+ ntohl(i->key));
+ if (publ) {
+ tipc_nodesub_unsubscribe(&publ->subscr);
+ kfree(publ);
+ }
+ } else {
+ pr_warn("Unrecognized name table message received\n");
+ }
+ return publ;
+}
+
+/**
* tipc_named_rcv - process name table update message sent by another node
*/
void tipc_named_rcv(struct sk_buff *buf)
{
- struct publication *publ;
struct tipc_msg *msg = buf_msg(buf);
struct distr_item *item = (struct distr_item *)msg_data(msg);
u32 count = msg_data_sz(msg) / ITEM_SIZE;
write_lock_bh(&tipc_nametbl_lock);
while (count--) {
- if (msg_type(msg) == PUBLICATION) {
- publ = tipc_nametbl_insert_publ(ntohl(item->type),
- ntohl(item->lower),
- ntohl(item->upper),
- TIPC_CLUSTER_SCOPE,
- msg_orignode(msg),
- ntohl(item->ref),
- ntohl(item->key));
- if (publ) {
- tipc_nodesub_subscribe(&publ->subscr,
- msg_orignode(msg),
- publ,
- (net_ev_handler)
- named_purge_publ);
- }
- } else if (msg_type(msg) == WITHDRAWAL) {
- publ = tipc_nametbl_remove_publ(ntohl(item->type),
- ntohl(item->lower),
- msg_orignode(msg),
- ntohl(item->ref),
- ntohl(item->key));
-
- if (publ) {
- tipc_nodesub_unsubscribe(&publ->subscr);
- kfree(publ);
- } else {
- pr_err("Unable to remove publication by node 0x%x\n"
- " (type=%u, lower=%u, ref=%u, key=%u)\n",
- msg_orignode(msg), ntohl(item->type),
- ntohl(item->lower), ntohl(item->ref),
- ntohl(item->key));
- }
- } else {
- pr_warn("Unrecognized name table message received\n");
- }
+ tipc_update_nametbl(item, msg_orignode(msg),
+ msg_type(msg));
item++;
}
write_unlock_bh(&tipc_nametbl_lock);
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 5/5] bnx2x: Fix timesync endianity
From: Yuval Mintz @ 2014-08-25 14:48 UTC (permalink / raw)
To: davem, netdev; +Cc: Ariel.Elior, Michal Kalderon, Yuval Mintz
In-Reply-To: <1408978113-27902-1-git-send-email-Yuval.Mintz@qlogic.com>
From: Michal Kalderon <Michal.Kalderon@qlogic.com>
Commit eeed018cbfa30 ("bnx2x: Add timestamping and PTP hardware clock support")
has a missing conversion to LE32, which will prevent the feature from working
on big endian machines.
Signed-off-by: Michal Kalderon <Michal.Kalderon@qlogic.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
index 798e97f..38acecd 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
@@ -5877,8 +5877,10 @@ int bnx2x_func_send_set_timesync(struct bnx2x *bp,
set_timesync_params->add_sub_drift_adjust_value;
rdata->drift_adjust_value = set_timesync_params->drift_adjust_value;
rdata->drift_adjust_period = set_timesync_params->drift_adjust_period;
- rdata->offset_delta.lo = U64_LO(set_timesync_params->offset_delta);
- rdata->offset_delta.hi = U64_HI(set_timesync_params->offset_delta);
+ rdata->offset_delta.lo =
+ cpu_to_le32(U64_LO(set_timesync_params->offset_delta));
+ rdata->offset_delta.hi =
+ cpu_to_le32(U64_HI(set_timesync_params->offset_delta));
DP(BNX2X_MSG_SP, "Set timesync command params: drift_cmd = %d, offset_cmd = %d, add_sub_drift = %d, drift_val = %d, drift_period = %d, offset_lo = %d, offset_hi = %d\n",
rdata->drift_adjust_cmd, rdata->offset_cmd,
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/5] bnx2x: Be more forgiving toward SW GRO
From: Yuval Mintz @ 2014-08-25 14:48 UTC (permalink / raw)
To: davem, netdev; +Cc: Ariel.Elior, Dmitry Kravkov, Yuval Mintz
In-Reply-To: <1408978113-27902-1-git-send-email-Yuval.Mintz@qlogic.com>
From: Dmitry Kravkov <Dmitry.Kravkov@qlogic.com>
This introduces 2 new relaxations in the bnx2x driver regarding GRO:
1. Don't prevent SW GRO if HW GRO is disabled.
2. If all aggregations are disabled, when GRO configuration changes
there's no need to perform an inner-reload [since it will have no
actual effect].
Signed-off-by: Dmitry Kravkov <Dmitry.Kravkov@qlogic.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 6304da2..de65640 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4816,11 +4816,15 @@ netdev_features_t bnx2x_fix_features(struct net_device *dev,
struct bnx2x *bp = netdev_priv(dev);
/* TPA requires Rx CSUM offloading */
- if (!(features & NETIF_F_RXCSUM) || bp->disable_tpa) {
+ if (!(features & NETIF_F_RXCSUM)) {
features &= ~NETIF_F_LRO;
features &= ~NETIF_F_GRO;
}
+ /* Note: do not disable SW GRO in kernel when HW GRO is off */
+ if (bp->disable_tpa)
+ features &= ~NETIF_F_LRO;
+
return features;
}
@@ -4859,6 +4863,10 @@ int bnx2x_set_features(struct net_device *dev, netdev_features_t features)
if ((changes & GRO_ENABLE_FLAG) && (flags & TPA_ENABLE_FLAG))
changes &= ~GRO_ENABLE_FLAG;
+ /* if GRO is changed while HW TPA is off, don't force a reload */
+ if ((changes & GRO_ENABLE_FLAG) && bp->disable_tpa)
+ changes &= ~GRO_ENABLE_FLAG;
+
if (changes)
bnx2x_reload = true;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/5] bnx2x: VF clean statistics
From: Yuval Mintz @ 2014-08-25 14:48 UTC (permalink / raw)
To: davem, netdev; +Cc: Ariel.Elior, Yuval Mintz
In-Reply-To: <1408978113-27902-1-git-send-email-Yuval.Mintz@qlogic.com>
During statistics initialization of a VF we need to clean its statistics.
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index e1c8193..d160829 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -1629,6 +1629,11 @@ void bnx2x_stats_init(struct bnx2x *bp)
int /*abs*/port = BP_PORT(bp);
int mb_idx = BP_FW_MB_IDX(bp);
+ if (IS_VF(bp)) {
+ bnx2x_memset_stats(bp);
+ return;
+ }
+
bp->stats_pending = 0;
bp->executer_idx = 0;
bp->stats_counter = 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/5] bnx2x: Fix stop-on-error
From: Yuval Mintz @ 2014-08-25 14:48 UTC (permalink / raw)
To: davem, netdev; +Cc: Ariel.Elior, Yuval Mintz
In-Reply-To: <1408978113-27902-1-git-send-email-Yuval.Mintz@qlogic.com>
When STOP_ON_ERROR is set driver will not compile. Even if it did,
traffic will not pass without this patch as several fields which are
verified by FW/HW on the Tx path are not properly set.
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 25 +++++++++++++++++++-----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 +-
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 2388144..2f210ba 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3873,12 +3873,16 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* when transmitting in a vf, start bd must hold the ethertype
* for fw to enforce it
*/
+#ifndef BNX2X_STOP_ON_ERROR
if (IS_VF(bp))
+#endif
tx_start_bd->vlan_or_ethertype =
cpu_to_le16(ntohs(eth->h_proto));
+#ifndef BNX2X_STOP_ON_ERROR
else
/* used by FW for packet accounting */
tx_start_bd->vlan_or_ethertype = cpu_to_le16(pkt_prod);
+#endif
}
nbd = 2; /* start_bd + pbd + frags (updated when pages are mapped) */
@@ -3951,11 +3955,22 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
&pbd_e2->data.mac_addr.dst_mid,
&pbd_e2->data.mac_addr.dst_lo,
eth->h_dest);
- } else if (bp->flags & TX_SWITCHING) {
- bnx2x_set_fw_mac_addr(&pbd_e2->data.mac_addr.dst_hi,
- &pbd_e2->data.mac_addr.dst_mid,
- &pbd_e2->data.mac_addr.dst_lo,
- eth->h_dest);
+ } else {
+ if (bp->flags & TX_SWITCHING)
+ bnx2x_set_fw_mac_addr(
+ &pbd_e2->data.mac_addr.dst_hi,
+ &pbd_e2->data.mac_addr.dst_mid,
+ &pbd_e2->data.mac_addr.dst_lo,
+ eth->h_dest);
+#ifdef BNX2X_STOP_ON_ERROR
+ /* Enforce security is always set in Stop on Error -
+ * source mac should be present in the parsing BD
+ */
+ bnx2x_set_fw_mac_addr(&pbd_e2->data.mac_addr.src_hi,
+ &pbd_e2->data.mac_addr.src_mid,
+ &pbd_e2->data.mac_addr.src_lo,
+ eth->h_source);
+#endif
}
SET_FLAG(pbd_e2_parsing_data,
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index a008f48..cf5b3b4 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -1142,7 +1142,7 @@ void bnx2x_panic_dump(struct bnx2x *bp, bool disable_int)
if (!fp->txdata_ptr)
break;
- if (!txdata.tx_cons_sb)
+ if (!txdata->tx_cons_sb)
continue;
start = TX_BD(le16_to_cpu(*txdata->tx_cons_sb) - 10);
--
1.8.3.1
^ permalink raw reply related
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