* Re: [PATCH 1/1 net-next] Bluetooth: hidp: replace kzalloc/copy_from_user by memdup_user
From: Marcel Holtmann @ 2014-11-15 0:31 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-kernel, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth, netdev
In-Reply-To: <1415990105-28609-1-git-send-email-fabf@skynet.be>
Hi Fabian,
> use memdup_user for rd_data import.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> net/bluetooth/hidp/core.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-15 0:24 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CA+mtBx9Z33O0x4-MG=66Fb4qvBJfxV54xOyiYvWydr=4Bka2xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 14, 2014 at 4:06 PM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>>
>>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>>> question about this API:
>>>>>>>>>
>>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>>> non-actively-steered flow? If the flow is subject to RFS, then moving
>>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>>> socket's CPU might move as well. The current implementation in this
>>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>>> which is not necessarily very useful.
>>>>>>>>
>>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>>> its cache. This is all that matters,
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Some possibilities:
>>>>>>>>>
>>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>>
>>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>>
>>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>>
>>>>>>>>
>>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>>
>>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>>> is what is currently returned). That would provide the complete
>>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>>
>>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>>> out where the flow would go without RFS. Can the network stack do
>>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>>> days)?
>>>>>>
>>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>>> the device for the socket. The former we already have, the latter
>>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>>> is where packet would have landed with RFS off.
>>>>
>>>> Hmm. I think this would be useful for me. It would *definitely* be
>>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>>
>>> Andy, can you elaborate a little more on your use case. I've thought
>>> several times about an interface to program the flow table from
>>> userspace, but never quite came up with a compelling use case and
>>> there is the security concern that a user could "steal" cycles from
>>> arbitrary CPUs.
>>
>> I have a bunch of threads that are pinned to various CPUs or groups of
>> CPUs. Each thread is responsible for a fixed set of flows. I'd like
>> those flows to go to those CPUs.
>>
>> RFS will eventually do it, but it would be nice if I could
>> deterministically ask for a flow to be routed to the right CPU. Also,
>> if my thread bounces temporarily to another CPU, I don't really need
>> the flow to follow it -- I'd like it to stay put.
>>
> Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
> called on a socket we can lock the socket to CPU binding to the
> current CPU it is called from. It could be unlocked at a later point.
> Would this satisfy your requirements?
Yes, I think. Especially if it bypassed the hash table.
>
>> This has a significant benefit over using automatic steering: with
>> automatic steering, I have to make all of the hash tables have a size
>> around the square of the total number of the flows in order to make it
>> reliable.
>>
>> Something like SO_STEER_TO_THIS_CPU would be fine, as long as it
>> reported whether it worked (for my diagnostics).
>>
>>>
>>>> With SO_INCOMING_CPU as described, I'm worried that people will write
>>>> programs that perform very well if RFS is off, but that once that code
>>>> runs with RFS on, weird things could happen.
>>>>
>>>> (On a side note: the RFS flow hash stuff seems to be rather buggy.
>>>> Some Solarflare engineers know about this, but a fix seems to be
>>>> rather slow in the works. I think that some of the bugs are in core
>>>> code, though.)
>>>
>>> This is problems with accelerated RFS or just getting the flow hash for packets?
>>
>> Accelerated RFS.
>>
>> Digging through my email, I thought that
>> net.core.rps_sock_flow_entries != the per-queue rps_flow_cnt would
>> make no sense, although I haven't configured it that way.
>>
>> More importantly, though, I think that some of the has table stuff is
>> problematic. My understanding is:
>>
>> get_rps_cpu may call set_rps_cpu, passing rflow =
>> flow_table->flows[hash & flow_table->mask];
>>
>> set_rps_cpu will compute flow_id = hash & flow_table->mask, which
>> looks to me like it has the property that rflow == flow_table[hash]
>> (unless we race with a hash table resize).
>>
>> Now set_rps_cpu tries to steer the new flow to the right CPU (all good
>> so far), but then it gets weird. We have a very high probability of
>> old_flow == rflow. rflow->filter gets overwritten with the filter id,
>> the if condition doesn't execute, and nothing gets set to
>> RPS_NO_FILTER.
>>
>> This is technically all correct, but if there are two active flows
>> with the same hash, they'll each keep getting steered to the same
>> place. This wastes cycles and seems to anger the sfc driver (the
>> latter is presumably an sfc bug). It also means that some of the
>> filters are likely to get expired for no good reason.
>>
> Yes, I could see where persistent hash collisions could cause an issue
> with aRFS. IIRC, I saw programming the filters to be quite an
> expensive operation. Minimally if seems like there some rate limiting
> change to avoid hysteresis.
They're especially expensive, given that they often generate printks
for me due to the sfc bug.
> As I mentioned, there is no material functionality in this patch and
> it should be independent of RFS. It simply returns the CPU where the
> stack processed the packet. Whether or not this is meaningful
> information to the algorithm being implemented in userspace is
> completely up to the caller to decide.
Agreed.
My only concern is that writing that userspace algorithm might result
in surprises if RFS is on. Having the user program notice the problem
early and alert the admin might help keep Murphy's Law at bay here.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-15 0:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrVQqsJKqVU4ENEfi86wkQLQ=mqBif=HcQiKT2h_fj22xQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 14, 2014 at 2:18 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Nov 14, 2014 at 2:16 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Fri, 2014-11-14 at 12:16 -0800, Andy Lutomirski wrote:
>>
>>> Sure, but how do I know that it'll be the same CPU next time?
>>
>> Because the NIC always use same RX queue for a given flow.
>>
>> So if you setup your IRQ affinities properly, the same CPU will drain
>> packets from this RX queue. And since RFS is off, you have the guarantee
>> the same CPU will be used to process packets in TCP stack.
>
> Right. My concern is that if RFS is off, everything works fine, but
> if RFS is on (and the app has no good way to know), then silly results
> will happen. I think I'd rather have the getsockopt fail if RFS is on
> or at least give some indication so that the app can react
> accordingly.
>
As I mentioned, there is no material functionality in this patch and
it should be independent of RFS. It simply returns the CPU where the
stack processed the packet. Whether or not this is meaningful
information to the algorithm being implemented in userspace is
completely up to the caller to decide.
Tom
> --Andy
>
>>
>> This SO_INCOMING_CPU info is a hint, there is no guarantee eg if you use
>> bonding and some load balancer or switch decides to send packets on
>> different links.
>>
>> Most NIC use Toeplitz hash, so given the 4-tuple, and rss key (40
>> bytes), you can actually compute the hash in software and know on which
>> RX queue traffic should land.
>>
>>
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-15 0:06 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrXf8j52nmpw-A2+bQt0yoz-fiD-4GP4Qf-afwH6UjCVnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>
>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>> question about this API:
>>>>>>>>
>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>> non-actively-steered flow? If the flow is subject to RFS, then moving
>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>> socket's CPU might move as well. The current implementation in this
>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>> which is not necessarily very useful.
>>>>>>>
>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>> its cache. This is all that matters,
>>>>>>>
>>>>>>>>
>>>>>>>> Some possibilities:
>>>>>>>>
>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>
>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>
>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>
>>>>>>>
>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>
>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>> is what is currently returned). That would provide the complete
>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>
>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>> out where the flow would go without RFS. Can the network stack do
>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>> days)?
>>>>>
>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>> the device for the socket. The former we already have, the latter
>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>> is where packet would have landed with RFS off.
>>>
>>> Hmm. I think this would be useful for me. It would *definitely* be
>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>
>> Andy, can you elaborate a little more on your use case. I've thought
>> several times about an interface to program the flow table from
>> userspace, but never quite came up with a compelling use case and
>> there is the security concern that a user could "steal" cycles from
>> arbitrary CPUs.
>
> I have a bunch of threads that are pinned to various CPUs or groups of
> CPUs. Each thread is responsible for a fixed set of flows. I'd like
> those flows to go to those CPUs.
>
> RFS will eventually do it, but it would be nice if I could
> deterministically ask for a flow to be routed to the right CPU. Also,
> if my thread bounces temporarily to another CPU, I don't really need
> the flow to follow it -- I'd like it to stay put.
>
Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
called on a socket we can lock the socket to CPU binding to the
current CPU it is called from. It could be unlocked at a later point.
Would this satisfy your requirements?
> This has a significant benefit over using automatic steering: with
> automatic steering, I have to make all of the hash tables have a size
> around the square of the total number of the flows in order to make it
> reliable.
>
> Something like SO_STEER_TO_THIS_CPU would be fine, as long as it
> reported whether it worked (for my diagnostics).
>
>>
>>> With SO_INCOMING_CPU as described, I'm worried that people will write
>>> programs that perform very well if RFS is off, but that once that code
>>> runs with RFS on, weird things could happen.
>>>
>>> (On a side note: the RFS flow hash stuff seems to be rather buggy.
>>> Some Solarflare engineers know about this, but a fix seems to be
>>> rather slow in the works. I think that some of the bugs are in core
>>> code, though.)
>>
>> This is problems with accelerated RFS or just getting the flow hash for packets?
>
> Accelerated RFS.
>
> Digging through my email, I thought that
> net.core.rps_sock_flow_entries != the per-queue rps_flow_cnt would
> make no sense, although I haven't configured it that way.
>
> More importantly, though, I think that some of the has table stuff is
> problematic. My understanding is:
>
> get_rps_cpu may call set_rps_cpu, passing rflow =
> flow_table->flows[hash & flow_table->mask];
>
> set_rps_cpu will compute flow_id = hash & flow_table->mask, which
> looks to me like it has the property that rflow == flow_table[hash]
> (unless we race with a hash table resize).
>
> Now set_rps_cpu tries to steer the new flow to the right CPU (all good
> so far), but then it gets weird. We have a very high probability of
> old_flow == rflow. rflow->filter gets overwritten with the filter id,
> the if condition doesn't execute, and nothing gets set to
> RPS_NO_FILTER.
>
> This is technically all correct, but if there are two active flows
> with the same hash, they'll each keep getting steered to the same
> place. This wastes cycles and seems to anger the sfc driver (the
> latter is presumably an sfc bug). It also means that some of the
> filters are likely to get expired for no good reason.
>
Yes, I could see where persistent hash collisions could cause an issue
with aRFS. IIRC, I saw programming the filters to be quite an
expensive operation. Minimally if seems like there some rate limiting
change to avoid hysteresis.
> Also, shouldn't ndo_rx_flow_steer be passed the full hash, not just
> the index into the hash table? What's the point of cutting off the
> high bits?
>
In order to make aRFS transparent to the user, it was implemented to
be driven from the RFS table which hashes based on the mask so that is
why it is limited. An alternative would be to program the filters
directly from a socket using the full hash, that interface might
already exist in ntuple filtering I suppose.
> --Andy
>
> --Andy
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 23:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1416007946.17262.84.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, Nov 14, 2014 at 3:32 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2014-11-14 at 15:03 -0800, Andy Lutomirski wrote:
>
>> If the kernel had an API for this, I'd be all for using it.
>
> It would be user land code, not kernel.
>
> Why doing a system call when you can avoid it ? ;)
>
> Doing it in the kernel would be quite complex actually.
A semi-official library would work, too. As long as it kept working.
>
> In userland, you can even implement this by machine learning.
>
> For every connection you made, you get the 4-tuple and INCOMING_CPU,
> then you store it in a cache.
Eww :(
>
> Next time you need to connect to same remote peer, you can lookup in the
> cache to find a good candidate.
>
> It would actually be good if we could do in a single socket verb a
> bind_and_connect(fd, &source, &destination)
Fair enough. Although for my use case, the time it takes to connect
is completely irrelevant.
--Andy
>
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 23:32 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrUAd50g2mYEOKT-5pEqMvwSstfQcgU8=7GRsO1XcKBSFA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-11-14 at 15:03 -0800, Andy Lutomirski wrote:
> If the kernel had an API for this, I'd be all for using it.
It would be user land code, not kernel.
Why doing a system call when you can avoid it ? ;)
Doing it in the kernel would be quite complex actually.
In userland, you can even implement this by machine learning.
For every connection you made, you get the 4-tuple and INCOMING_CPU,
then you store it in a cache.
Next time you need to connect to same remote peer, you can lookup in the
cache to find a good candidate.
It would actually be good if we could do in a single socket verb a
bind_and_connect(fd, &source, &destination)
^ permalink raw reply
* [PATCH] ieee802154: fix error handling in ieee802154fake_probe()
From: Alexey Khoroshilov @ 2014-11-14 23:11 UTC (permalink / raw)
To: Alexander Aring, David S . Miller
Cc: Alexey Khoroshilov, linux-wpan, netdev, linux-kernel, ldv-project
In case of any failure ieee802154fake_probe() just calls unregister_netdev().
But it does not look safe to unregister netdevice before it was registered.
The patch implements straightforward resource deallocation in case of
failure in ieee802154fake_probe().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/ieee802154/fakehard.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ieee802154/fakehard.c b/drivers/net/ieee802154/fakehard.c
index 9ce854f43917..6cbc56ad9ff4 100644
--- a/drivers/net/ieee802154/fakehard.c
+++ b/drivers/net/ieee802154/fakehard.c
@@ -377,17 +377,20 @@ static int ieee802154fake_probe(struct platform_device *pdev)
err = wpan_phy_register(phy);
if (err)
- goto out;
+ goto err_phy_reg;
err = register_netdev(dev);
- if (err < 0)
- goto out;
+ if (err)
+ goto err_netdev_reg;
dev_info(&pdev->dev, "Added ieee802154 HardMAC hardware\n");
return 0;
-out:
- unregister_netdev(dev);
+err_netdev_reg:
+ wpan_phy_unregister(phy);
+err_phy_reg:
+ free_netdev(dev);
+ wpan_phy_free(phy);
return err;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 23:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1416005912.17262.76.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, Nov 14, 2014 at 2:58 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2014-11-14 at 14:27 -0800, Andy Lutomirski wrote:
>
>> The people at the other end will be really pissed if that results in
>> lots of reconnections.
>
> No reconnections necessary.
>
> I believe you misunderstood : On the 4-tuple (SADDR,SPORT,DADDR,DPORT),
> you can pick for example SPORT so that hash(SADDR,SPORT,DADDR,DPORT)
> maps to a known and wanted RX queue number.
>
> Once you know that, you use bind(SADDR, SPORT), then
> connect(DADDR,DPORT).
If the kernel had an API for this, I'd be all for using it.
>
> Anyway, if your hardware is able to cope with the few number of flows,
> just use the hardware and be happy.
>
> Here we want about 10 millions sockets, there is little hope for
> hardware being helpful.
>
It's the intermediate numbers that are bad.
With ten flows, the current accelerated RFS works fine. With 10M
flows, RFS is a lost cause and this solution is much nicer. With,
say, 1k flows, accelerated RFS *deserves* to work perfectly, because
the hardware has enough filter slots. But making it work reliably
requires a ridiculously large hash table, and collisions cause silent
bad behavior.
--Andy
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: tcp_mark_head_lost warning in Linux kernel 3.10
From: Eric Dumazet @ 2014-11-14 23:00 UTC (permalink / raw)
To: Vinson Lee, Neal Cardwell
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <CAHTgTXXqaVzVOyGuVWpdQokY8RmYRMriaLhBecLzfg=HF6V8dQ@mail.gmail.com>
On Fri, 2014-11-14 at 14:39 -0800, Vinson Lee wrote:
> Hi.
>
> We hit this networking tcp_mark_head_lost warning in Linux kernel 3.10.
>
CC Neal, because he is working on this
> ------------[ cut here ]------------
> WARNING: at net/ipv4/tcp_input.c:2262 tcp_mark_head_lost+0x1aa/0x1dd()
> Modules linked in: netconsole configfs xt_DSCP iptable_mangle
> cpufreq_ondemand ipv6 ppdev parport_pc lp parport tcp_diag inet_diag
> ipmi_si ipmi_devintf ipmi_msghandler iTCO_wdt iTCO_vendor_support
> acpi_cpufreq freq_table mperf coretemp kvm_intel kvm crc32c_intel
> ghash_clmulni_intel microcode serio_raw i2c_i801 lpc_ich mfd_core igb
> i2c_algo_bit i2c_core ptp pps_core ioatdma dca wmi
> CPU: 1 PID: 16561 Not tainted 3.10.50 #1
> 0000000000000000 ffff88085fc23948 ffffffff814a4094 ffff88085fc23980
> ffffffff8103cbf9 0000000000000000 ffff8807b644b200 0000000000000001
> ffff880764e1ca40 000000004e9c4970 ffff88085fc23990 ffffffff8103ccbf
> Call Trace:
> <IRQ> [<ffffffff814a4094>] dump_stack+0x19/0x1b
> [<ffffffff8103cbf9>] warn_slowpath_common+0x65/0x7d
> [<ffffffff8103ccbf>] warn_slowpath_null+0x1a/0x1c
> [<ffffffff81434c61>] tcp_mark_head_lost+0x1aa/0x1dd
> [<ffffffff814367fc>] tcp_update_scoreboard+0x4f/0x13f
> [<ffffffff81438beb>] tcp_fastretrans_alert+0x736/0x7be
> [<ffffffff81439644>] tcp_ack+0x95e/0xb47
> [<ffffffff81439be6>] tcp_rcv_established+0xde/0x442
> [<ffffffff81442cab>] tcp_v4_do_rcv+0x1e6/0x3e8
> [<ffffffff8144371e>] tcp_v4_rcv+0x293/0x52c
> [<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
> [<ffffffff8142d562>] ? __inet_lookup_listener+0xfa/0x17f
> [<ffffffff81425f4a>] ip_local_deliver_finish+0xe0/0x155
> [<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
> [<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
> [<ffffffff814260ef>] ip_local_deliver+0x40/0x52
> [<ffffffff81425d50>] ip_rcv_finish+0x274/0x2b6
> [<ffffffff81425adc>] ? inet_del_offload+0x3d/0x3d
> [<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
> [<ffffffff81426324>] ip_rcv+0x223/0x267
> [<ffffffff813f76b4>] __netif_receive_skb_core+0x435/0x4a7
> [<ffffffff8107bab8>] ? timekeeping_get_ns.constprop.10+0x11/0x36
> [<ffffffff813f773e>] __netif_receive_skb+0x18/0x5a
> [<ffffffff813f8813>] netif_receive_skb+0x40/0x75
> [<ffffffff813f8f94>] napi_gro_receive+0x3e/0x80
> [<ffffffffa004a097>] igb_clean_rx_irq+0x67e/0x69e [igb]
> [<ffffffffa004a425>] igb_poll+0x36e/0x5b0 [igb]
> [<ffffffff813ea49a>] ? __sk_mem_reclaim+0x2f/0x84
> [<ffffffff8143ef76>] ? tcp_delack_timer_handler+0x2e/0x19c
> [<ffffffff813f8a61>] net_rx_action+0xcf/0x196
> [<ffffffff8106870c>] ? sched_clock_cpu+0x42/0xc7
> [<ffffffff8104371d>] __do_softirq+0xd5/0x1f4
> [<ffffffff814b01bc>] call_softirq+0x1c/0x30
> [<ffffffff81003ecd>] do_softirq+0x33/0x6e
> [<ffffffff81043931>] irq_exit+0x51/0x93
> [<ffffffff814b086e>] do_IRQ+0x8e/0xa5
> [<ffffffff814a85aa>] common_interrupt+0x6a/0x6a
> <EOI>
> ---[ end trace 8b2ed2c5c443e39d ]---
>
>
> net/ipv4/tcp_input.c
> 2200 /* Detect loss in event "A" above by marking head of queue up as lost.
> 2201 * For FACK or non-SACK(Reno) senders, the first "packets"
> number of segments
> 2202 * are considered lost. For RFC3517 SACK, a segment is
> considered lost if it
> 2203 * has at least tp->reordering SACKed seqments above it;
> "packets" refers to
> 2204 * the maximum SACKed segments to pass before reaching this limit.
> 2205 */
> 2206 static void tcp_mark_head_lost(struct sock *sk, int packets,
> int mark_head)
> 2207 {
> [...]
> 2262 tcp_verify_left_out(tp);
> 2263 }
>
> include/net/tcp.h
> 936 /* Use define here intentionally to get WARN_ON location shown
> at the caller */
> 937 #define tcp_verify_left_out(tp) WARN_ON(tcp_left_out(tp) >
> tp->packets_out)
>
> Cheers,
> Vinson
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 22:58 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrWhsj+byMeT=WHb9eLR_vgsUe58Li8JDLaTvsSPWp1DKQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-11-14 at 14:27 -0800, Andy Lutomirski wrote:
> The people at the other end will be really pissed if that results in
> lots of reconnections.
No reconnections necessary.
I believe you misunderstood : On the 4-tuple (SADDR,SPORT,DADDR,DPORT),
you can pick for example SPORT so that hash(SADDR,SPORT,DADDR,DPORT)
maps to a known and wanted RX queue number.
Once you know that, you use bind(SADDR, SPORT), then
connect(DADDR,DPORT).
Anyway, if your hardware is able to cope with the few number of flows,
just use the hardware and be happy.
Here we want about 10 millions sockets, there is little hope for
hardware being helpful.
^ permalink raw reply
* tcp_mark_head_lost warning in Linux kernel 3.10
From: Vinson Lee @ 2014-11-14 22:39 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: netdev
Hi.
We hit this networking tcp_mark_head_lost warning in Linux kernel 3.10.
------------[ cut here ]------------
WARNING: at net/ipv4/tcp_input.c:2262 tcp_mark_head_lost+0x1aa/0x1dd()
Modules linked in: netconsole configfs xt_DSCP iptable_mangle
cpufreq_ondemand ipv6 ppdev parport_pc lp parport tcp_diag inet_diag
ipmi_si ipmi_devintf ipmi_msghandler iTCO_wdt iTCO_vendor_support
acpi_cpufreq freq_table mperf coretemp kvm_intel kvm crc32c_intel
ghash_clmulni_intel microcode serio_raw i2c_i801 lpc_ich mfd_core igb
i2c_algo_bit i2c_core ptp pps_core ioatdma dca wmi
CPU: 1 PID: 16561 Not tainted 3.10.50 #1
0000000000000000 ffff88085fc23948 ffffffff814a4094 ffff88085fc23980
ffffffff8103cbf9 0000000000000000 ffff8807b644b200 0000000000000001
ffff880764e1ca40 000000004e9c4970 ffff88085fc23990 ffffffff8103ccbf
Call Trace:
<IRQ> [<ffffffff814a4094>] dump_stack+0x19/0x1b
[<ffffffff8103cbf9>] warn_slowpath_common+0x65/0x7d
[<ffffffff8103ccbf>] warn_slowpath_null+0x1a/0x1c
[<ffffffff81434c61>] tcp_mark_head_lost+0x1aa/0x1dd
[<ffffffff814367fc>] tcp_update_scoreboard+0x4f/0x13f
[<ffffffff81438beb>] tcp_fastretrans_alert+0x736/0x7be
[<ffffffff81439644>] tcp_ack+0x95e/0xb47
[<ffffffff81439be6>] tcp_rcv_established+0xde/0x442
[<ffffffff81442cab>] tcp_v4_do_rcv+0x1e6/0x3e8
[<ffffffff8144371e>] tcp_v4_rcv+0x293/0x52c
[<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
[<ffffffff8142d562>] ? __inet_lookup_listener+0xfa/0x17f
[<ffffffff81425f4a>] ip_local_deliver_finish+0xe0/0x155
[<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
[<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
[<ffffffff814260ef>] ip_local_deliver+0x40/0x52
[<ffffffff81425d50>] ip_rcv_finish+0x274/0x2b6
[<ffffffff81425adc>] ? inet_del_offload+0x3d/0x3d
[<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
[<ffffffff81426324>] ip_rcv+0x223/0x267
[<ffffffff813f76b4>] __netif_receive_skb_core+0x435/0x4a7
[<ffffffff8107bab8>] ? timekeeping_get_ns.constprop.10+0x11/0x36
[<ffffffff813f773e>] __netif_receive_skb+0x18/0x5a
[<ffffffff813f8813>] netif_receive_skb+0x40/0x75
[<ffffffff813f8f94>] napi_gro_receive+0x3e/0x80
[<ffffffffa004a097>] igb_clean_rx_irq+0x67e/0x69e [igb]
[<ffffffffa004a425>] igb_poll+0x36e/0x5b0 [igb]
[<ffffffff813ea49a>] ? __sk_mem_reclaim+0x2f/0x84
[<ffffffff8143ef76>] ? tcp_delack_timer_handler+0x2e/0x19c
[<ffffffff813f8a61>] net_rx_action+0xcf/0x196
[<ffffffff8106870c>] ? sched_clock_cpu+0x42/0xc7
[<ffffffff8104371d>] __do_softirq+0xd5/0x1f4
[<ffffffff814b01bc>] call_softirq+0x1c/0x30
[<ffffffff81003ecd>] do_softirq+0x33/0x6e
[<ffffffff81043931>] irq_exit+0x51/0x93
[<ffffffff814b086e>] do_IRQ+0x8e/0xa5
[<ffffffff814a85aa>] common_interrupt+0x6a/0x6a
<EOI>
---[ end trace 8b2ed2c5c443e39d ]---
net/ipv4/tcp_input.c
2200 /* Detect loss in event "A" above by marking head of queue up as lost.
2201 * For FACK or non-SACK(Reno) senders, the first "packets"
number of segments
2202 * are considered lost. For RFC3517 SACK, a segment is
considered lost if it
2203 * has at least tp->reordering SACKed seqments above it;
"packets" refers to
2204 * the maximum SACKed segments to pass before reaching this limit.
2205 */
2206 static void tcp_mark_head_lost(struct sock *sk, int packets,
int mark_head)
2207 {
[...]
2262 tcp_verify_left_out(tp);
2263 }
include/net/tcp.h
936 /* Use define here intentionally to get WARN_ON location shown
at the caller */
937 #define tcp_verify_left_out(tp) WARN_ON(tcp_left_out(tp) >
tp->packets_out)
Cheers,
Vinson
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 22:37 UTC (permalink / raw)
To: Jay Vosburgh
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <18948.1416003002@famine>
Hi Jay,
On Fr, 2014-11-14 at 14:10 -0800, Jay Vosburgh wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> [...]
> >I created it via the function calling convention documented in
> >arch/x86/include/asm/calling.h, so I specified each register which a
> >function is allowed to clobber with.
> >
> >I currently cannot see how I can resolve the invalid constraints error
> >easily. :(
> >
> >So either go with my first patch, which I puts the alternative_call
> >switch point into its own function without ever inlining or the patch
> >needs to be reverted. :/
>
> As a data point, I tested the first patch as well, and the
> system does not panic with it in place. Inspection shows that it's
> using %r14 in place of %r8 in the prior (crashing) implementation.
Yes, I also could reproduce your oops and the first unoffical patch and
the first offical one both fixed it. After that, I thought that just
adding more clobbers cannot introduce bugs, so I only did compile
testing until I hit a window where gcc got mad with the excessive use of
clobbered registers but haven't tested the inline call sites that much
(sorry). :(
> Disassembly of the call site (on the non-sse4_1 system) in
> ovs_flow_tbl_insert with the first patch applied looks like this:
>
> 0xffffffffa00b6bb9 <ovs_flow_tbl_insert+0xb9>: mov %r15,0x348(%r14)
> 0xffffffffa00b6bc0 <ovs_flow_tbl_insert+0xc0>: movzwl 0x28(%r15),%ecx
> 0xffffffffa00b6bc5 <ovs_flow_tbl_insert+0xc5>: movzwl 0x2a(%r15),%esi
> 0xffffffffa00b6bca <ovs_flow_tbl_insert+0xca>: movzwl %cx,%eax
> 0xffffffffa00b6bcd <ovs_flow_tbl_insert+0xcd>: sub %ecx,%esi
> 0xffffffffa00b6bcf <ovs_flow_tbl_insert+0xcf>: lea 0x38(%r14,%rax,1),%rdi
> 0xffffffffa00b6bd4 <ovs_flow_tbl_insert+0xd4>: sar $0x2,%esi
> 0xffffffffa00b6bd7 <ovs_flow_tbl_insert+0xd7>: callq 0xffffffff813a7810 <__jhash2>
> 0xffffffffa00b6bdc <ovs_flow_tbl_insert+0xdc>: mov %eax,0x30(%r14)
> 0xffffffffa00b6be0 <ovs_flow_tbl_insert+0xe0>: mov (%rbx),%r13
> 0xffffffffa00b6be3 <ovs_flow_tbl_insert+0xe3>: mov %r14,%rsi
> 0xffffffffa00b6be6 <ovs_flow_tbl_insert+0xe6>: mov %r13,%rdi
> 0xffffffffa00b6be9 <ovs_flow_tbl_insert+0xe9>: callq 0xffffffffa00b61a0 <table_instance_insert>
>
> Compared to the panicking version's function:
>
> 0xffffffffa01a55c9 <ovs_flow_tbl_insert+0xb9>: mov %r15,0x348(%r8)
> 0xffffffffa01a55d0 <ovs_flow_tbl_insert+0xc0>: movzwl 0x28(%r15),%ecx
> 0xffffffffa01a55d5 <ovs_flow_tbl_insert+0xc5>: movzwl 0x2a(%r15),%esi
> 0xffffffffa01a55da <ovs_flow_tbl_insert+0xca>: movzwl %cx,%eax
> 0xffffffffa01a55dd <ovs_flow_tbl_insert+0xcd>: sub %ecx,%esi
> 0xffffffffa01a55df <ovs_flow_tbl_insert+0xcf>: lea 0x38(%r8,%rax,1),%rdi
> 0xffffffffa01a55e4 <ovs_flow_tbl_insert+0xd4>: sar $0x2,%esi
> 0xffffffffa01a55e7 <ovs_flow_tbl_insert+0xd7>: callq 0xffffffff813a75c0 <__jhash2>
> 0xffffffffa01a55ec <ovs_flow_tbl_insert+0xdc>: mov %eax,0x30(%r8)
> 0xffffffffa01a55f0 <ovs_flow_tbl_insert+0xe0>: mov (%rbx),%r13
> 0xffffffffa01a55f3 <ovs_flow_tbl_insert+0xe3>: mov %r8,%rsi
> 0xffffffffa01a55f6 <ovs_flow_tbl_insert+0xe6>: mov %r13,%rdi
> 0xffffffffa01a55f9 <ovs_flow_tbl_insert+0xe9>: callq 0xffffffffa01a4ba0 <table_instance_insert>
>
> It appears to generate the same instructions, but allocates
> registers differently (using %r14 instead of %r8).
Exactly and that makes sense. While %r8 must be available for the callee
to be clobbered with, %r14 must be saved by the callee and restored
before returning. So you pass the responsibility down to the other
functions, which tries not to touch %r14 because it knows it will have
to generate code for saving and restoring.
That's the reason why I actually like the the static inline clobbering
approach so much, it gives gcc possibilities to move around the
save/restore cycles and decide itself just by aligning which registers
to use.
Also the first version does work flawlessly (which I didn't send as a
patch but only as a diff in the mail). Here gcc synthesizes a full
function call which has the same effect as the long clobber list, only
it does chain two calls right behind each other.
> The __jhash2 disassembly appears to be unchanged between the two
> versions.
Thanks for looking into this!
It is actually pretty hairy to come up with a good solution for this,
because with the alternative interface you are only allowed to alter one
instruction. jump_tables also don't work because I currently have the
opinion that they do the switch way too late. I absolutely don't want to
have inserts into a hashtable with different hashing functions depending
how early during boot they took place.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 22:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1416003883.17262.72.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, Nov 14, 2014 at 2:24 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2014-11-14 at 14:10 -0800, Andy Lutomirski wrote:
>
>> I have a bunch of threads that are pinned to various CPUs or groups of
>> CPUs. Each thread is responsible for a fixed set of flows. I'd like
>> those flows to go to those CPUs.
>>
>> RFS will eventually do it, but it would be nice if I could
>> deterministically ask for a flow to be routed to the right CPU. Also,
>> if my thread bounces temporarily to another CPU, I don't really need
>> the flow to follow it -- I'd like it to stay put.
>>
>> This has a significant benefit over using automatic steering: with
>> automatic steering, I have to make all of the hash tables have a size
>> around the square of the total number of the flows in order to make it
>> reliable.
>>
>> Something like SO_STEER_TO_THIS_CPU would be fine, as long as it
>> reported whether it worked (for my diagnostics).
>
> This requires some kind of hardware support, and unfortunately this is
> not generic.
>
> With SO_INCOMING_CPU, you simply can pass fd of sockets around threads,
> so that a dumb RSS multiqueue NIC is OK (assuming you are not using some
> encapsulation that NIC is not able to parse to find L4 information)
I can't really do this. It means that the performance of my system
will be wildly different every time I restart it. I don't have enough
connections for everything to average out.
>
> Steering is a dream, I really think its easier to build flows so that
> their RX queue matches your requirements.
I have supporting hardware :) I just want it to work without
programming the ntuple table myself.
>
> We usually can pick at least one element of the 4-tuple, so its actually
> possible to get this before connect().
>
Hmm. An API for that would be quite nice :)
>
> Two cases :
>
> 1) Passive connections.
>
> After accept(), get SO_INCOMING_CPU, then pass the fd to appropriate
> thread of your pool.
>
> 2) Active connections .
> find a proper 4-tuple, bind() then connect(). Eventually check
> SO_INCOMING_CPU to verify your expectations.
The people at the other end will be really pissed if that results in
lots of reconnections.
--Andy
>
>
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 22:24 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrXf8j52nmpw-A2+bQt0yoz-fiD-4GP4Qf-afwH6UjCVnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-11-14 at 14:10 -0800, Andy Lutomirski wrote:
> I have a bunch of threads that are pinned to various CPUs or groups of
> CPUs. Each thread is responsible for a fixed set of flows. I'd like
> those flows to go to those CPUs.
>
> RFS will eventually do it, but it would be nice if I could
> deterministically ask for a flow to be routed to the right CPU. Also,
> if my thread bounces temporarily to another CPU, I don't really need
> the flow to follow it -- I'd like it to stay put.
>
> This has a significant benefit over using automatic steering: with
> automatic steering, I have to make all of the hash tables have a size
> around the square of the total number of the flows in order to make it
> reliable.
>
> Something like SO_STEER_TO_THIS_CPU would be fine, as long as it
> reported whether it worked (for my diagnostics).
This requires some kind of hardware support, and unfortunately this is
not generic.
With SO_INCOMING_CPU, you simply can pass fd of sockets around threads,
so that a dumb RSS multiqueue NIC is OK (assuming you are not using some
encapsulation that NIC is not able to parse to find L4 information)
Steering is a dream, I really think its easier to build flows so that
their RX queue matches your requirements.
We usually can pick at least one element of the 4-tuple, so its actually
possible to get this before connect().
Two cases :
1) Passive connections.
After accept(), get SO_INCOMING_CPU, then pass the fd to appropriate
thread of your pool.
2) Active connections .
find a proper 4-tuple, bind() then connect(). Eventually check
SO_INCOMING_CPU to verify your expectations.
^ permalink raw reply
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Florian Fainelli @ 2014-11-14 22:23 UTC (permalink / raw)
To: Martin K. Petersen, netdev; +Cc: linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
On 11/14/2014 11:16 AM, Martin K. Petersen wrote:
[snip]
> +#ifdef CONFIG_OF
> +/**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> + struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> + struct ixgbe_hw *hw = &adapter->hw;
> + const unsigned char *addr;
> + int len;
> +
> + addr = of_get_property(dp, "local-mac-address", &len);
> + if (addr && len == 6) {
> + e_dev_info("Using OpenPROM MAC address\n");
> + memcpy(hw->mac.perm_addr, addr, 6);
> + }
Cannot you use of_get_mac_address() here which does iterate through all
the OF standard ways to specify a MAC address: mac-address,
local-mac-address and address? Benefit is that this will work for all
DT-enabled platforms.
> +
> + if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> + e_dev_info("Using IDPROM MAC address\n");
> + memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
> + }
> +}
> +#endif
> +
> /**
> * ixgbe_probe - Device Initialization Routine
> * @pdev: PCI device information struct
> @@ -8223,6 +8253,10 @@ skip_sriov:
> goto err_sw_init;
> }
>
> +#ifdef CONFIG_OF
> + ixgbe_of_mac_addr(adapter);
> +#endif
> +
> memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
>
> if (!is_valid_ether_addr(netdev->dev_addr)) {
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 22:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1416003371.17262.66.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, Nov 14, 2014 at 2:16 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2014-11-14 at 12:16 -0800, Andy Lutomirski wrote:
>
>> Sure, but how do I know that it'll be the same CPU next time?
>
> Because the NIC always use same RX queue for a given flow.
>
> So if you setup your IRQ affinities properly, the same CPU will drain
> packets from this RX queue. And since RFS is off, you have the guarantee
> the same CPU will be used to process packets in TCP stack.
Right. My concern is that if RFS is off, everything works fine, but
if RFS is on (and the app has no good way to know), then silly results
will happen. I think I'd rather have the getsockopt fail if RFS is on
or at least give some indication so that the app can react
accordingly.
--Andy
>
> This SO_INCOMING_CPU info is a hint, there is no guarantee eg if you use
> bonding and some load balancer or switch decides to send packets on
> different links.
>
> Most NIC use Toeplitz hash, so given the 4-tuple, and rss key (40
> bytes), you can actually compute the hash in software and know on which
> RX queue traffic should land.
>
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 22:16 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Tom Herbert, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrUuChgxXcNrKjYzpe1ry0hx3kf19EfAbY1N1YnG7NFZUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-11-14 at 12:16 -0800, Andy Lutomirski wrote:
> Sure, but how do I know that it'll be the same CPU next time?
Because the NIC always use same RX queue for a given flow.
So if you setup your IRQ affinities properly, the same CPU will drain
packets from this RX queue. And since RFS is off, you have the guarantee
the same CPU will be used to process packets in TCP stack.
This SO_INCOMING_CPU info is a hint, there is no guarantee eg if you use
bonding and some load balancer or switch decides to send packets on
different links.
Most NIC use Toeplitz hash, so given the 4-tuple, and rss key (40
bytes), you can actually compute the hash in software and know on which
RX queue traffic should land.
^ permalink raw reply
* Re: [PATCHv2 net 0/4] Implement ndo_gso_check() for vxlan nics
From: David Miller @ 2014-11-14 22:13 UTC (permalink / raw)
To: joestringer
Cc: netdev, sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev,
therbert, gerlitz.or, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>
From: Joe Stringer <joestringer@nicira.com>
Date: Thu, 13 Nov 2014 16:38:11 -0800
> Most NICs that report NETIF_F_GSO_UDP_TUNNEL support VXLAN, and not other
> UDP-based encapsulation protocols where the format and size of the header may
> differ. This patch series implements a generic ndo_gso_check() for detecting
> VXLAN, then reuses it for these NICs.
>
> Implementation shamelessly stolen from Tom Herbert (with minor fixups):
> http://thread.gmane.org/gmane.linux.network/332428/focus=333111
>
> v2: Drop i40e/fm10k patches (code diverged; handling separately).
> Refactor common code into vxlan_gso_check() helper.
> Minor style fixes.
Series applied, thanks Joe.
^ permalink raw reply
* [PATCH] brcmfmac: fix error handling of irq_of_parse_and_map
From: Dmitry Torokhov @ 2014-11-14 22:12 UTC (permalink / raw)
To: John W. Linville
Cc: Brett Rudley, Arend van Spriel, Franky (Zhenhui) Lin,
Hante Meuleman, Pieter-Paul Giesberts, linux-wireless,
brcm80211-dev-list, netdev, linux-kernel
Return value of irq_of_parse_and_map() is unsigned int, with 0
indicating failure, so testing for negative result never works.
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
---
Not tested, found by casual code inspection.
drivers/net/wireless/brcm80211/brcmfmac/of.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/of.c b/drivers/net/wireless/brcm80211/brcmfmac/of.c
index eb3fce82..c824570 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/of.c
@@ -40,8 +40,8 @@ void brcmf_of_probe(struct brcmf_sdio_dev *sdiodev)
return;
irq = irq_of_parse_and_map(np, 0);
- if (irq < 0) {
- brcmf_err("interrupt could not be mapped: err=%d\n", irq);
+ if (!irq) {
+ brcmf_err("interrupt could not be mapped\n");
devm_kfree(dev, sdiodev->pdata);
return;
}
--
2.1.0.rc2.206.gedb03e5
--
Dmitry
^ permalink raw reply related
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 22:10 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CA+mtBx9dSNOD5Z8GinU8F1mj_PgU0sTC5g8S9vRuSnSUzAoD=g@mail.gmail.com>
On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert@google.com> wrote:
> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert@google.com> wrote:
>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert@google.com> wrote:
>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>
>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>> question about this API:
>>>>>>>
>>>>>>> How does an application tell whether the socket represents a
>>>>>>> non-actively-steered flow? If the flow is subject to RFS, then moving
>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>> socket's CPU might move as well. The current implementation in this
>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>> which is not necessarily very useful.
>>>>>>
>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>> its cache. This is all that matters,
>>>>>>
>>>>>>>
>>>>>>> Some possibilities:
>>>>>>>
>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>
>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>
>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>
>>>>>>
>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>> obviously logic needed in user space that would need to do something.
>>>>>
>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>> reported time), and the CPU processing transport (post steering which
>>>>> is what is currently returned). That would provide the complete
>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>
>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>> out where the flow would go without RFS. Can the network stack do
>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>> days)?
>>>>
>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>> the device for the socket. The former we already have, the latter
>>> might be done by adding a field to skbuff to set received CPU. Given
>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>> is where packet would have landed with RFS off.
>>
>> Hmm. I think this would be useful for me. It would *definitely* be
>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>
> Andy, can you elaborate a little more on your use case. I've thought
> several times about an interface to program the flow table from
> userspace, but never quite came up with a compelling use case and
> there is the security concern that a user could "steal" cycles from
> arbitrary CPUs.
I have a bunch of threads that are pinned to various CPUs or groups of
CPUs. Each thread is responsible for a fixed set of flows. I'd like
those flows to go to those CPUs.
RFS will eventually do it, but it would be nice if I could
deterministically ask for a flow to be routed to the right CPU. Also,
if my thread bounces temporarily to another CPU, I don't really need
the flow to follow it -- I'd like it to stay put.
This has a significant benefit over using automatic steering: with
automatic steering, I have to make all of the hash tables have a size
around the square of the total number of the flows in order to make it
reliable.
Something like SO_STEER_TO_THIS_CPU would be fine, as long as it
reported whether it worked (for my diagnostics).
>
>> With SO_INCOMING_CPU as described, I'm worried that people will write
>> programs that perform very well if RFS is off, but that once that code
>> runs with RFS on, weird things could happen.
>>
>> (On a side note: the RFS flow hash stuff seems to be rather buggy.
>> Some Solarflare engineers know about this, but a fix seems to be
>> rather slow in the works. I think that some of the bugs are in core
>> code, though.)
>
> This is problems with accelerated RFS or just getting the flow hash for packets?
Accelerated RFS.
Digging through my email, I thought that
net.core.rps_sock_flow_entries != the per-queue rps_flow_cnt would
make no sense, although I haven't configured it that way.
More importantly, though, I think that some of the has table stuff is
problematic. My understanding is:
get_rps_cpu may call set_rps_cpu, passing rflow =
flow_table->flows[hash & flow_table->mask];
set_rps_cpu will compute flow_id = hash & flow_table->mask, which
looks to me like it has the property that rflow == flow_table[hash]
(unless we race with a hash table resize).
Now set_rps_cpu tries to steer the new flow to the right CPU (all good
so far), but then it gets weird. We have a very high probability of
old_flow == rflow. rflow->filter gets overwritten with the filter id,
the if condition doesn't execute, and nothing gets set to
RPS_NO_FILTER.
This is technically all correct, but if there are two active flows
with the same hash, they'll each keep getting steered to the same
place. This wastes cycles and seems to anger the sfc driver (the
latter is presumably an sfc bug). It also means that some of the
filters are likely to get expired for no good reason.
Also, shouldn't ndo_rx_flow_steer be passed the full hash, not just
the index into the hash table? What's the point of cutting off the
high bits?
--Andy
--Andy
^ permalink raw reply
* Re: pull request: wireless 2014-11-13
From: David Miller @ 2014-11-14 22:10 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20141113212815.GA12075@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 13 Nov 2014 16:28:16 -0500
> Please pull this set of a few more wireless fixes intended for the
> 3.18 stream...
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Jay Vosburgh @ 2014-11-14 22:10 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <1415997309.15154.59.camel@localhost>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
[...]
>I created it via the function calling convention documented in
>arch/x86/include/asm/calling.h, so I specified each register which a
>function is allowed to clobber with.
>
>I currently cannot see how I can resolve the invalid constraints error
>easily. :(
>
>So either go with my first patch, which I puts the alternative_call
>switch point into its own function without ever inlining or the patch
>needs to be reverted. :/
As a data point, I tested the first patch as well, and the
system does not panic with it in place. Inspection shows that it's
using %r14 in place of %r8 in the prior (crashing) implementation.
Disassembly of the call site (on the non-sse4_1 system) in
ovs_flow_tbl_insert with the first patch applied looks like this:
0xffffffffa00b6bb9 <ovs_flow_tbl_insert+0xb9>: mov %r15,0x348(%r14)
0xffffffffa00b6bc0 <ovs_flow_tbl_insert+0xc0>: movzwl 0x28(%r15),%ecx
0xffffffffa00b6bc5 <ovs_flow_tbl_insert+0xc5>: movzwl 0x2a(%r15),%esi
0xffffffffa00b6bca <ovs_flow_tbl_insert+0xca>: movzwl %cx,%eax
0xffffffffa00b6bcd <ovs_flow_tbl_insert+0xcd>: sub %ecx,%esi
0xffffffffa00b6bcf <ovs_flow_tbl_insert+0xcf>: lea 0x38(%r14,%rax,1),%rdi
0xffffffffa00b6bd4 <ovs_flow_tbl_insert+0xd4>: sar $0x2,%esi
0xffffffffa00b6bd7 <ovs_flow_tbl_insert+0xd7>: callq 0xffffffff813a7810 <__jhash2>
0xffffffffa00b6bdc <ovs_flow_tbl_insert+0xdc>: mov %eax,0x30(%r14)
0xffffffffa00b6be0 <ovs_flow_tbl_insert+0xe0>: mov (%rbx),%r13
0xffffffffa00b6be3 <ovs_flow_tbl_insert+0xe3>: mov %r14,%rsi
0xffffffffa00b6be6 <ovs_flow_tbl_insert+0xe6>: mov %r13,%rdi
0xffffffffa00b6be9 <ovs_flow_tbl_insert+0xe9>: callq 0xffffffffa00b61a0 <table_instance_insert>
Compared to the panicking version's function:
0xffffffffa01a55c9 <ovs_flow_tbl_insert+0xb9>: mov %r15,0x348(%r8)
0xffffffffa01a55d0 <ovs_flow_tbl_insert+0xc0>: movzwl 0x28(%r15),%ecx
0xffffffffa01a55d5 <ovs_flow_tbl_insert+0xc5>: movzwl 0x2a(%r15),%esi
0xffffffffa01a55da <ovs_flow_tbl_insert+0xca>: movzwl %cx,%eax
0xffffffffa01a55dd <ovs_flow_tbl_insert+0xcd>: sub %ecx,%esi
0xffffffffa01a55df <ovs_flow_tbl_insert+0xcf>: lea 0x38(%r8,%rax,1),%rdi
0xffffffffa01a55e4 <ovs_flow_tbl_insert+0xd4>: sar $0x2,%esi
0xffffffffa01a55e7 <ovs_flow_tbl_insert+0xd7>: callq 0xffffffff813a75c0 <__jhash2>
0xffffffffa01a55ec <ovs_flow_tbl_insert+0xdc>: mov %eax,0x30(%r8)
0xffffffffa01a55f0 <ovs_flow_tbl_insert+0xe0>: mov (%rbx),%r13
0xffffffffa01a55f3 <ovs_flow_tbl_insert+0xe3>: mov %r8,%rsi
0xffffffffa01a55f6 <ovs_flow_tbl_insert+0xe6>: mov %r13,%rdi
0xffffffffa01a55f9 <ovs_flow_tbl_insert+0xe9>: callq 0xffffffffa01a4ba0 <table_instance_insert>
It appears to generate the same instructions, but allocates
registers differently (using %r14 instead of %r8).
The __jhash2 disassembly appears to be unchanged between the two
versions.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH] inetdevice: fixed signed integer overflow
From: David Miller @ 2014-11-14 22:09 UTC (permalink / raw)
To: vincent.benayoun; +Cc: jiri, ja, nicolas.dichtel, linux-kernel, netdev
In-Reply-To: <1e488fa546724140b55c8d4bd6b5a9d6@S1688.EX1688.lan>
From: Vincent Benayoun <vincent.benayoun@trust-in-soft.com>
Date: Thu, 13 Nov 2014 15:04:57 +0000
> From f25ff0f2645f9763552147d480f86973b7041e26 Mon Sep 17 00:00:00 2001
> From: Vincent BENAYOUN <vincent.benayoun@trust-in-soft.com>
> Date: Thu, 13 Nov 2014 13:47:26 +0100
> Subject: [PATCH] inetdevice: fixed signed integer overflow
>
> There could be a signed overflow in the following code.
>
> The expression, (32-logmask) is comprised between 0 and 31 included.
> It may be equal to 31.
> In such a case the left shift will produce a signed integer overflow.
> According to the C99 Standard, this is an undefined behavior.
> A simple fix is to replace the signed int 1 with the unsigned int 1U.
>
> Signed-off-by: Vincent BENAYOUN <vincent.benayoun@trust-in-soft.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] Revert "fast_hash: avoid indirect function calls"
From: Hannes Frederic Sowa @ 2014-11-14 21:43 UTC (permalink / raw)
To: Jay Vosburgh
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <16481.1415991906@famine>
On Fr, 2014-11-14 at 11:05 -0800, Jay Vosburgh wrote:
> This reverts commit e5a2c899957659cd1a9f789bc462f9c0b35f5150.
>
> Commit e5a2c899 introduced an alternative_call, arch_fast_hash2,
> that selects between __jhash2 and __intel_crc4_2_hash based on the
> X86_FEATURE_XMM4_2.
>
> Unfortunately, the alternative_call system does not appear to be
> suitable for use with C functions, as register usage is not handled
> properly for the called functions. The __jhash2 function in particular
> clobbers registers that are not preserved when called via
> alternative_call, resulting in a panic for direct callers of
> arch_fast_hash2 on older CPUs lacking sse4_2. It is possible that
> __intel_crc4_2_hash works merely by chance because it uses fewer
> registers.
>
> This commit was suggested as the source of the problem by Jesse
> Gross <jesse@nicira.com>.
>
> Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
I am totally fine to revert this and try to come up with a better
solution.
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Bye,
Hannes
^ permalink raw reply
* Re: [linux-nics] [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Jeff Kirsher @ 2014-11-14 21:40 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Sergei Shtylyov, David Miller, Jesse Brandeburg, netdev,
linux.nics
In-Reply-To: <yq1mw7tv8jq.fsf@sermon.lab.mkp.net>
[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]
On Fri, 2014-11-14 at 15:20 -0500, Martin K. Petersen wrote:
> >>>>> "Sergei" == Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> writes:
>
> >> +#ifdef CONFIG_OF
> >> + ixgbe_of_mac_addr(adapter);
> >> +#endif
>
> Sergei> Eww... why not define the following above instead:
>
> Sergei> #else
> Sergei> static inline void ixgbe_of_mac_addr(struct ixgbe_adapter
> Sergei> *adapter) {}
> Sergei> #endif
>
> I don't care much either way. But we might as well do this, then, and
> shave off an ifdef...
>
> commit 01e25f145972563ee87ebf85b7cb02a4ff8fce3b
> Author: Martin K. Petersen <martin.petersen@oracle.com>
> Date: Wed Nov 12 20:47:42 2014 -0500
>
> ixgbe: Look up MAC address in Open Firmware
>
> Attempt to look up the MAC address in Open Firmware on systems
> that
> support it. If the "local-mac-address" property is not valid
> resort to
> using the IDPROM value.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Was this meant to be a v2 patch??? Yikes!
Based on the feedback from Segei, Dave and Jesse. Dropping this patch
from my queue and will wait for a properly formatted v2 of this patch.
Please CC me on any future patches against any changes to
drivers/net/ethernet/intel/*
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox