* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Joe Perches @ 2014-11-14 19:14 UTC (permalink / raw)
To: Fabian Frederick; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <2104595537.56209.1415991758726.open-xchange@webmail.nmp.skynet.be>
On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > kmalloc_array manages count*sizeof overflow.
> >
> > Fundamentally correct, but is this necessary or useful?
> > sizeof(s8) isn't often going to be anything other than 1.
> Absolutely, I thought it was a struct :)
>
> There must be a reason for so many cases though ...
Some might be style symmetry for other sizeof(othertype)
uses in the same paths, but most of them are just overkill
or maybe lack of understanding.
s8 is char so by definition it has to be 1.
I doubt any of the code dates from PDP-8/TOPS-10 days.
^ permalink raw reply
* [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Martin K. Petersen @ 2014-11-14 19:16 UTC (permalink / raw)
To: netdev; +Cc: linux.nics
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>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3d1032..365924124fab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,11 @@
#include <linux/prefetch.h>
#include <scsi/fc/fc_fcoe.h>
+#ifdef CONFIG_OF
+#include <asm/idprom.h>
+#include <asm/prom.h>
+#endif
+
#include "ixgbe.h"
#include "ixgbe_common.h"
#include "ixgbe_dcb_82599.h"
@@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
return is_wol_supported;
}
+#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);
+ }
+
+ 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)) {
^ permalink raw reply related
* Re: [linux-nics] [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Jeff Kirsher @ 2014-11-14 19:26 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: netdev, linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
On Fri, 2014-11-14 at 14:16 -0500, Martin K. Petersen wrote:
>
> 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>
Thanks Martin, I will add your patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Fabian Frederick @ 2014-11-14 19:32 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <1415992486.5912.23.camel@perches.com>
> On 14 November 2014 at 20:14 Joe Perches <joe@perches.com> wrote:
>
>
> On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> > On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > > kmalloc_array manages count*sizeof overflow.
> > >
> > > Fundamentally correct, but is this necessary or useful?
> > > sizeof(s8) isn't often going to be anything other than 1.
> > Absolutely, I thought it was a struct :)
> >
> > There must be a reason for so many cases though ...
>
> Some might be style symmetry for other sizeof(othertype)
> uses in the same paths, but most of them are just overkill
> or maybe lack of understanding.
>
> s8 is char so by definition it has to be 1.
>
> I doubt any of the code dates from PDP-8/TOPS-10 days.
>
>
95% comes from drivers tree. I guess one patch to Greg would be enough.
Are you interested in those patches or can I do them with some "Suggested-by" ?
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 19:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Michael Kerrisk, David Miller, netdev, Ying Cai, Willem de Bruijn,
Neal Cardwell, Linux API
In-Reply-To: <CALCETrXmCWSoeQLC3WCqMf_sehGVE9KTbTGmsA77ZN84B-V24Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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.
RFS is the other way around : You want the flow to follow your thread.
RPS wont be a problem if you have sensible RPS settings.
>
> 2. Change the interface a bit to report the socket's preferred CPU
> (where it would go without RFS, for example) and then let the
> application use setsockopt to tell the socket to stay put (i.e. turn
> off RFS and RPS for that flow).
>
> 3. Report the preferred CPU as in (2) but let the application ask for
> something different.
>
> For example, I have flows for which I know which CPU I want. A nice
> API to put the flow there would be quite useful.
>
>
> Also, it may be worth changing the naming to indicate that these are
> about the rx cpu (they are, right?). For some applications (sparse,
> low-latency flows, for example), it can be useful to keep the tx
> completion handling on a different CPU.
SO_INCOMING_CPU is rx, like incoming ;)
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Joe Perches @ 2014-11-14 19:42 UTC (permalink / raw)
To: Fabian Frederick; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <47990659.56515.1415993549555.open-xchange@webmail.nmp.skynet.be>
On Fri, 2014-11-14 at 20:32 +0100, Fabian Frederick wrote:
> On 14 November 2014 at 20:14 Joe Perches <joe@perches.com> wrote:
> > On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> > > On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > > > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > > > kmalloc_array manages count*sizeof overflow.
> > > >
> > > > Fundamentally correct, but is this necessary or useful?
> > > > sizeof(s8) isn't often going to be anything other than 1.
> > > Absolutely, I thought it was a struct :)
> > >
> > > There must be a reason for so many cases though ...
> >
> > Some might be style symmetry for other sizeof(othertype)
> > uses in the same paths, but most of them are just overkill
> > or maybe lack of understanding.
> 95% comes from drivers tree. I guess one patch to Greg would be enough.
I think it'd be better to send patches through the
appropriate various maintainers
Likely just using the 2nd level directory would be
good enough
$ git grep -E --name-only "\*\s*sizeof\s*\(\s*[us]8\s*\)" | \
cut -f1-2 -d"/" | uniq
arch/arm
drivers/acpi
drivers/char
drivers/gpu
drivers/iio
drivers/infiniband
drivers/input
drivers/md
drivers/media
drivers/mtd
drivers/net
drivers/power
drivers/rtc
drivers/thermal
fs/compat_ioctl.c
net/dsa
> Are you interested in those patches or can I do them with some "Suggested-by" ?
Not really and no need.
cheers, Joe
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-14 19:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andy Lutomirski, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1415993614.17262.55.camel@edumazet-glaptop2.roam.corp.google.com>
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.
>
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.
> RFS is the other way around : You want the flow to follow your thread.
>
> RPS wont be a problem if you have sensible RPS settings.
>
>>
>> 2. Change the interface a bit to report the socket's preferred CPU
>> (where it would go without RFS, for example) and then let the
>> application use setsockopt to tell the socket to stay put (i.e. turn
>> off RFS and RPS for that flow).
>>
>> 3. Report the preferred CPU as in (2) but let the application ask for
>> something different.
>>
>> For example, I have flows for which I know which CPU I want. A nice
>> API to put the flow there would be quite useful.
>>
>>
>> Also, it may be worth changing the naming to indicate that these are
>> about the rx cpu (they are, right?). For some applications (sparse,
>> low-latency flows, for example), it can be useful to keep the tx
>> completion handling on a different CPU.
>
> SO_INCOMING_CPU is rx, like incoming ;)
>
>
> --
> 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] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:04 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, netdev, ogerlitz, pshelar, jesse, jay.vosburgh,
discuss
In-Reply-To: <20141114.133829.1437047454714311242.davem@davemloft.net>
On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Fri, 14 Nov 2014 16:46:18 +0100
>
> > I would still like to see the current proposed fix getting applied and
> > we can do this on-top. The inline call after this patch reassembles a
> > direct function call, so besides the long list of clobbers, it should
> > still be pretty fast.
>
> I would rather revert the change entirely until it is implemented
> properly.
>
> Also, I am strongly of the opinion that this is a mis-use of the
> alternative call interface. It was never intended to be used for
> things that can make real function calls.
I tend to disagree. Grepping e.g. shows
alternative_call_2(copy_user_generic_unrolled,
copy_user_generic_string,
X86_FEATURE_REP_GOOD,
copy_user_enhanced_fast_string,
X86_FEATURE_ERMS,
ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
"=d" (len)),
"1" (to), "2" (from), "3" (len)
: "memory", "rcx", "r8", "r9", "r10", "r11");
(it has a few less clobbers because it has more output operands)
I just tried to come up with some macros which lets you abstract away
the clobber list, but in the end it somehow has to look exactly like
that. The double-colon syntax also makes it difficult to come up with
something that let's us use varargs for that.
> You can add a million clobbers, or a trampoline, it's still using a
> facility in a manner for which it was not designed.
The full clobber list for a function call which would always clear
registers like we would have in a normal non-inlined function call would
look like this:
#define FUNC_CLOBBER LIST "memory", "cc", "rax", "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11"
(reference in arch/x86/include/asm/calling.h).
> This means a new interface with a new name and with capabilities
> explicitly supporting this case are in order.
It try to implicitly embed the clobber list, would something like that
be ok?
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Sergei Shtylyov @ 2014-11-14 20:07 UTC (permalink / raw)
To: Martin K. Petersen, netdev; +Cc: linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
Hello.
On 11/14/2014 10:16 PM, Martin K. Petersen wrote:
> 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>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d2df4e3d1032..365924124fab 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
[...]
> @@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
> return is_wol_supported;
> }
>
> +#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);
> + }
> +
> + 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
Eww... why not define the following above instead:
#else
static inline void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter) {}
#endif
This is closer to what Documentation/SubmittingPatches suggests.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:10 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, netdev, ogerlitz, pshelar, jesse, jay.vosburgh,
discuss
In-Reply-To: <1415995451.15154.54.camel@localhost>
On Fr, 2014-11-14 at 21:04 +0100, Hannes Frederic Sowa wrote:
> On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Fri, 14 Nov 2014 16:46:18 +0100
> >
> > > I would still like to see the current proposed fix getting applied and
> > > we can do this on-top. The inline call after this patch reassembles a
> > > direct function call, so besides the long list of clobbers, it should
> > > still be pretty fast.
> >
> > I would rather revert the change entirely until it is implemented
> > properly.
> >
> > Also, I am strongly of the opinion that this is a mis-use of the
> > alternative call interface. It was never intended to be used for
> > things that can make real function calls.
>
> I tend to disagree. Grepping e.g. shows
>
> alternative_call_2(copy_user_generic_unrolled,
> copy_user_generic_string,
> X86_FEATURE_REP_GOOD,
> copy_user_enhanced_fast_string,
> X86_FEATURE_ERMS,
> ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> "=d" (len)),
> "1" (to), "2" (from), "3" (len)
> : "memory", "rcx", "r8", "r9", "r10", "r11");
Oh, I need to take this back, These are actually assembler functions.
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Jay Vosburgh @ 2014-11-14 20:15 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <1415995451.15154.54.camel@localhost>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Date: Fri, 14 Nov 2014 16:46:18 +0100
>>
>> > I would still like to see the current proposed fix getting applied and
>> > we can do this on-top. The inline call after this patch reassembles a
>> > direct function call, so besides the long list of clobbers, it should
>> > still be pretty fast.
>>
>> I would rather revert the change entirely until it is implemented
>> properly.
>>
>> Also, I am strongly of the opinion that this is a mis-use of the
>> alternative call interface. It was never intended to be used for
>> things that can make real function calls.
>
>I tend to disagree. Grepping e.g. shows
>
> alternative_call_2(copy_user_generic_unrolled,
> copy_user_generic_string,
> X86_FEATURE_REP_GOOD,
> copy_user_enhanced_fast_string,
> X86_FEATURE_ERMS,
> ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> "=d" (len)),
> "1" (to), "2" (from), "3" (len)
> : "memory", "rcx", "r8", "r9", "r10", "r11");
>
>
>(it has a few less clobbers because it has more output operands)
As those functions (copy_user_generic_unrolled, et al) are all
in assembly language, presumably the list of clobbered registers can be
had via inspection.
For the arch_fast_hash2 case, the functions (__intel_crc4_2_hash
and __jash2) are both written in C, so how would the clobber list be
created?
-J
>I just tried to come up with some macros which lets you abstract away
>the clobber list, but in the end it somehow has to look exactly like
>that. The double-colon syntax also makes it difficult to come up with
>something that let's us use varargs for that.
>
>> You can add a million clobbers, or a trampoline, it's still using a
>> facility in a manner for which it was not designed.
>
>The full clobber list for a function call which would always clear
>registers like we would have in a normal non-inlined function call would
>look like this:
>
>#define FUNC_CLOBBER LIST "memory", "cc", "rax", "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11"
>
>(reference in arch/x86/include/asm/calling.h).
>
>> This means a new interface with a new name and with capabilities
>> explicitly supporting this case are in order.
>
>It try to implicitly embed the clobber list, would something like that
>be ok?
>
>Thanks,
>Hannes
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 20:16 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+mtBx_DyUCxE6BqL_xXcqKPUDu7b3jLShV+AL6L+gCzJLriPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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)?
>
>> RFS is the other way around : You want the flow to follow your thread.
>>
>> RPS wont be a problem if you have sensible RPS settings.
>>
>>>
>>> 2. Change the interface a bit to report the socket's preferred CPU
>>> (where it would go without RFS, for example) and then let the
>>> application use setsockopt to tell the socket to stay put (i.e. turn
>>> off RFS and RPS for that flow).
>>>
>>> 3. Report the preferred CPU as in (2) but let the application ask for
>>> something different.
>>>
>>> For example, I have flows for which I know which CPU I want. A nice
>>> API to put the flow there would be quite useful.
>>>
>>>
>>> Also, it may be worth changing the naming to indicate that these are
>>> about the rx cpu (they are, right?). For some applications (sparse,
>>> low-latency flows, for example), it can be useful to keep the tx
>>> completion handling on a different CPU.
>>
>> SO_INCOMING_CPU is rx, like incoming ;)
>>
>>
Duh :)
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Martin K. Petersen @ 2014-11-14 20:20 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Martin K. Petersen, netdev, linux.nics
In-Reply-To: <54666106.9000609@cogentembedded.com>
>>>>> "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>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3d1032..0e45a43172eb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,11 @@
#include <linux/prefetch.h>
#include <scsi/fc/fc_fcoe.h>
+#ifdef CONFIG_OF
+#include <asm/idprom.h>
+#include <asm/prom.h>
+#endif
+
#include "ixgbe.h"
#include "ixgbe_common.h"
#include "ixgbe_dcb_82599.h"
@@ -7960,6 +7965,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
}
/**
+ * 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)
+{
+#ifdef CONFIG_OF
+ 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);
+ }
+
+ 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
* @ent: entry in ixgbe_pci_tbl
@@ -8223,6 +8253,8 @@ skip_sriov:
goto err_sw_init;
}
+ ixgbe_of_mac_addr(adapter);
+
memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
if (!is_valid_ether_addr(netdev->dev_addr)) {
^ permalink raw reply related
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-14 20:25 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: <CALCETrUuChgxXcNrKjYzpe1ry0hx3kf19EfAbY1N1YnG7NFZUw@mail.gmail.com>
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.
>>
>>> RFS is the other way around : You want the flow to follow your thread.
>>>
>>> RPS wont be a problem if you have sensible RPS settings.
>>>
>>>>
>>>> 2. Change the interface a bit to report the socket's preferred CPU
>>>> (where it would go without RFS, for example) and then let the
>>>> application use setsockopt to tell the socket to stay put (i.e. turn
>>>> off RFS and RPS for that flow).
>>>>
>>>> 3. Report the preferred CPU as in (2) but let the application ask for
>>>> something different.
>>>>
>>>> For example, I have flows for which I know which CPU I want. A nice
>>>> API to put the flow there would be quite useful.
>>>>
>>>>
>>>> Also, it may be worth changing the naming to indicate that these are
>>>> about the rx cpu (they are, right?). For some applications (sparse,
>>>> low-latency flows, for example), it can be useful to keep the tx
>>>> completion handling on a different CPU.
>>>
>>> SO_INCOMING_CPU is rx, like incoming ;)
>>>
>>>
>
> Duh :)
>
>>> --
>>> 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
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 1/1 net-next] openvswitch: use PTR_ERR_OR_ZERO
From: Pravin Shelar @ 2014-11-14 20:34 UTC (permalink / raw)
To: Fabian Frederick; +Cc: LKML, David S. Miller, dev@openvswitch.org, netdev
In-Reply-To: <1415989978-28559-1-git-send-email-fabf@skynet.be>
On Fri, Nov 14, 2014 at 10:32 AM, Fabian Frederick <fabf@skynet.be> wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> net/openvswitch/flow_netlink.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index c0d066d..c8fccdd 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -1425,10 +1425,8 @@ static int add_action(struct sw_flow_actions **sfa, int attrtype,
> struct nlattr *a;
>
> a = __add_action(sfa, attrtype, data, len, log);
> - if (IS_ERR(a))
> - return PTR_ERR(a);
>
> - return 0;
> + return PTR_ERR_OR_ZERO(a);
> }
>
> static inline int add_nested_action_start(struct sw_flow_actions **sfa,
> --
> 1.9.3
>
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 20:34 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+mtBx_tNW_Bz0hovZ90XxcLK6pL8d6RX=v5Rg8utfKkaFoJ+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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.
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.)
--Andy
>
>>>
>>>> RFS is the other way around : You want the flow to follow your thread.
>>>>
>>>> RPS wont be a problem if you have sensible RPS settings.
>>>>
>>>>>
>>>>> 2. Change the interface a bit to report the socket's preferred CPU
>>>>> (where it would go without RFS, for example) and then let the
>>>>> application use setsockopt to tell the socket to stay put (i.e. turn
>>>>> off RFS and RPS for that flow).
>>>>>
>>>>> 3. Report the preferred CPU as in (2) but let the application ask for
>>>>> something different.
>>>>>
>>>>> For example, I have flows for which I know which CPU I want. A nice
>>>>> API to put the flow there would be quite useful.
>>>>>
>>>>>
>>>>> Also, it may be worth changing the naming to indicate that these are
>>>>> about the rx cpu (they are, right?). For some applications (sparse,
>>>>> low-latency flows, for example), it can be useful to keep the tx
>>>>> completion handling on a different CPU.
>>>>
>>>> SO_INCOMING_CPU is rx, like incoming ;)
>>>>
>>>>
>>
>> Duh :)
>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Andy Lutomirski
>> AMA Capital Management, LLC
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:35 UTC (permalink / raw)
To: Jay Vosburgh
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <17658.1415996115@famine>
On Fr, 2014-11-14 at 12:15 -0800, Jay Vosburgh wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>
> >On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> >> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >> Date: Fri, 14 Nov 2014 16:46:18 +0100
> >>
> >> > I would still like to see the current proposed fix getting applied and
> >> > we can do this on-top. The inline call after this patch reassembles a
> >> > direct function call, so besides the long list of clobbers, it should
> >> > still be pretty fast.
> >>
> >> I would rather revert the change entirely until it is implemented
> >> properly.
> >>
> >> Also, I am strongly of the opinion that this is a mis-use of the
> >> alternative call interface. It was never intended to be used for
> >> things that can make real function calls.
> >
> >I tend to disagree. Grepping e.g. shows
> >
> > alternative_call_2(copy_user_generic_unrolled,
> > copy_user_generic_string,
> > X86_FEATURE_REP_GOOD,
> > copy_user_enhanced_fast_string,
> > X86_FEATURE_ERMS,
> > ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> > "=d" (len)),
> > "1" (to), "2" (from), "3" (len)
> > : "memory", "rcx", "r8", "r9", "r10", "r11");
> >
> >
> >(it has a few less clobbers because it has more output operands)
>
> As those functions (copy_user_generic_unrolled, et al) are all
> in assembly language, presumably the list of clobbered registers can be
> had via inspection.
>
> For the arch_fast_hash2 case, the functions (__intel_crc4_2_hash
> and __jash2) are both written in C, so how would the clobber list be
> created?
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. :/
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:42 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Eric Dumazet, netdev, Or Gerlitz, Pravin B Shelar,
Jesse Gross, jay.vosburgh, discuss
In-Reply-To: <CAHA+R7NKrwE7F4k4wiaZRJ67QAhB9bUe8SdyuT8Ky3-wAou6cA@mail.gmail.com>
On Fr, 2014-11-14 at 11:02 -0800, Cong Wang wrote:
>
> I am wondering, compare with alternative call, how slower is just
> testing
> cpu_has_xmm4_2?
I can test, but cpu_has_xmm4_2 expands into quite some code. I don't
know if indirect function call or a test of this flag is faster. I'll
test this.
Bye,
Hannes
^ permalink raw reply
* [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)
From: Fabian Frederick @ 2014-11-14 20:55 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Stefano Brivio, John W. Linville, Johannes Berg,
Emmanuel Grumbach, Intel Linux Wireless, Larry Finger,
Chaoming Li, linux-wireless, b43-dev, netdev
sizeof(u8) is always 1.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
drivers/net/wireless/b43/ppr.c | 2 +-
drivers/net/wireless/iwlwifi/pcie/trans.c | 2 +-
drivers/net/wireless/rtlwifi/efuse.c | 16 ++++++++--------
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/b43/ppr.c b/drivers/net/wireless/b43/ppr.c
index 9a77027..6bc1c6f 100644
--- a/drivers/net/wireless/b43/ppr.c
+++ b/drivers/net/wireless/b43/ppr.c
@@ -28,7 +28,7 @@ void b43_ppr_clear(struct b43_wldev *dev, struct b43_ppr *ppr)
memset(ppr, 0, sizeof(*ppr));
/* Compile-time PPR check */
- BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM * sizeof(u8));
+ BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM);
}
void b43_ppr_add(struct b43_wldev *dev, struct b43_ppr *ppr, int diff)
diff --git a/drivers/net/wireless/iwlwifi/pcie/trans.c b/drivers/net/wireless/iwlwifi/pcie/trans.c
index 836725e..f016824 100644
--- a/drivers/net/wireless/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/iwlwifi/pcie/trans.c
@@ -1157,7 +1157,7 @@ static void iwl_trans_pcie_configure(struct iwl_trans *trans,
trans_pcie->n_no_reclaim_cmds = trans_cfg->n_no_reclaim_cmds;
if (trans_pcie->n_no_reclaim_cmds)
memcpy(trans_pcie->no_reclaim_cmds, trans_cfg->no_reclaim_cmds,
- trans_pcie->n_no_reclaim_cmds * sizeof(u8));
+ trans_pcie->n_no_reclaim_cmds);
trans_pcie->rx_buf_size_8k = trans_cfg->rx_buf_size_8k;
if (trans_pcie->rx_buf_size_8k)
diff --git a/drivers/net/wireless/rtlwifi/efuse.c b/drivers/net/wireless/rtlwifi/efuse.c
index 0b4082c..a3135c5 100644
--- a/drivers/net/wireless/rtlwifi/efuse.c
+++ b/drivers/net/wireless/rtlwifi/efuse.c
@@ -251,8 +251,8 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
}
/* allocate memory for efuse_tbl and efuse_word */
- efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
- sizeof(u8), GFP_ATOMIC);
+ efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE],
+ GFP_ATOMIC);
if (!efuse_tbl)
return;
efuse_word = kzalloc(EFUSE_MAX_WORD_UNIT * sizeof(u16 *), GFP_ATOMIC);
@@ -733,8 +733,8 @@ static int efuse_pg_packet_read(struct ieee80211_hw *hw, u8 offset, u8 *data)
if (offset > 15)
return false;
- memset(data, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
- memset(tmpdata, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
+ memset(data, 0xff, PGPKT_DATA_SIZE);
+ memset(tmpdata, 0xff, PGPKT_DATA_SIZE);
while (continual && (efuse_addr < EFUSE_MAX_SIZE)) {
if (readstate & PG_STATE_HEADER) {
@@ -772,7 +772,7 @@ static void efuse_write_data_case1(struct ieee80211_hw *hw, u16 *efuse_addr,
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct pgpkt_struct tmp_pkt;
int dataempty = true;
- u8 originaldata[8 * sizeof(u8)];
+ u8 originaldata[8];
u8 badworden = 0x0F;
u8 match_word_en, tmp_word_en;
u8 tmpindex;
@@ -881,7 +881,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,
struct pgpkt_struct tmp_pkt;
u8 pg_header;
u8 tmp_header;
- u8 originaldata[8 * sizeof(u8)];
+ u8 originaldata[8];
u8 tmp_word_cnts;
u8 badworden = 0x0F;
@@ -904,7 +904,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,
tmp_word_cnts = efuse_calculate_word_cnts(tmp_pkt.word_en);
- memset(originaldata, 0xff, 8 * sizeof(u8));
+ memset(originaldata, 0xff, 8);
if (efuse_pg_packet_read(hw, tmp_pkt.offset, originaldata)) {
badworden = enable_efuse_data_write(hw,
@@ -962,7 +962,7 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
target_pkt.offset = offset;
target_pkt.word_en = word_en;
- memset(target_pkt.data, 0xFF, 8 * sizeof(u8));
+ memset(target_pkt.data, 0xFF, 8);
efuse_word_enable_data_read(word_en, data, target_pkt.data);
target_word_cnts = efuse_calculate_word_cnts(target_pkt.word_en);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Jesse Brandeburg @ 2014-11-14 21:11 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: Sergei Shtylyov, netdev, linux.nics, jesse.brandeburg
In-Reply-To: <yq1mw7tv8jq.fsf@sermon.lab.mkp.net>
On Fri, 14 Nov 2014 15:20:09 -0500
"Martin K. Petersen" <martin.petersen@oracle.com> wrote:
> 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>
seems fine, but see below...
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d2df4e3d1032..0e45a43172eb 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -50,6 +50,11 @@
> #include <linux/prefetch.h>
> #include <scsi/fc/fc_fcoe.h>
>
> +#ifdef CONFIG_OF
> +#include <asm/idprom.h>
> +#include <asm/prom.h>
> +#endif
> +
> #include "ixgbe.h"
> #include "ixgbe_common.h"
> #include "ixgbe_dcb_82599.h"
> @@ -7960,6 +7965,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
> }
>
> /**
> + * 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)
> +{
> +#ifdef CONFIG_OF
> + 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);
> + }
> +
> + if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> + e_dev_info("Using IDPROM MAC address\n");
There really isn't much point to print this message, just delete this
e_dev_info line.
^ permalink raw reply
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: David Miller @ 2014-11-14 21:25 UTC (permalink / raw)
To: martin.petersen; +Cc: netdev, linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
From: "Martin K. Petersen" <martin.petersen@oracle.com>
Date: Fri, 14 Nov 2014 14:16:50 -0500
> +#ifdef CONFIG_OF
> +#include <asm/idprom.h>
> +#include <asm/prom.h>
> +#endif
CONFIG_OF doesn't imply the presence of idprom.
idprom is completely sparc/m68k specific, whereas OF exists on many
platforms other than sparc/m68k.
^ permalink raw reply
* Re: [PATCH Iproute2 next] ip link: Add ipvlan support to the iproute2/ip util
From: Mahesh Bandewar @ 2014-11-14 21:25 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Stephen Hemminger, Eric Dumazet, Maciej Zenczykowski,
Laurent Chavey, Tim Hockin, David Miller, Brandon Philips,
Pavel Emelianov
In-Reply-To: <20141114100959.GA19965@unicorn.suse.cz>
On Fri, Nov 14, 2014 at 2:09 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
> On Thu, Nov 13, 2014 at 10:56:51PM -0800, Mahesh Bandewar wrote:
>> Adding basic support to create virtual devices using 'ip'
>> utility. Following is the syntax -
>>
>> ip link add link <master> <virtual> mode [ l2 | l3 ]
>> e.g. ip link add link eth0 ipvl0 mode l3
>
> I suppose "type ipvlan" is missing on both lines.
>
yes it does, damn!
> Michal Kubecek
>
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: David Miller @ 2014-11-14 21:35 UTC (permalink / raw)
To: hannes
Cc: cwang, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
jay.vosburgh, discuss
In-Reply-To: <1415997733.15154.62.camel@localhost>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 14 Nov 2014 21:42:13 +0100
> On Fr, 2014-11-14 at 11:02 -0800, Cong Wang wrote:
>>
>> I am wondering, compare with alternative call, how slower is just
>> testing
>> cpu_has_xmm4_2?
>
> I can test, but cpu_has_xmm4_2 expands into quite some code. I don't
> know if indirect function call or a test of this flag is faster. I'll
> test this.
I'm going to apply Jay's revert, and then you guys can take your time
sorting this out.
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-14 21:36 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: <CALCETrWYrR2X9u7CAr5xkSg8kLH_O=sdk=JF+ZW+FxCTNTFBZg@mail.gmail.com>
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.
> 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?
Thanks,
Tom
>
> --Andy
>
>>
>>>>
>>>>> RFS is the other way around : You want the flow to follow your thread.
>>>>>
>>>>> RPS wont be a problem if you have sensible RPS settings.
>>>>>
>>>>>>
>>>>>> 2. Change the interface a bit to report the socket's preferred CPU
>>>>>> (where it would go without RFS, for example) and then let the
>>>>>> application use setsockopt to tell the socket to stay put (i.e. turn
>>>>>> off RFS and RPS for that flow).
>>>>>>
>>>>>> 3. Report the preferred CPU as in (2) but let the application ask for
>>>>>> something different.
>>>>>>
>>>>>> For example, I have flows for which I know which CPU I want. A nice
>>>>>> API to put the flow there would be quite useful.
>>>>>>
>>>>>>
>>>>>> Also, it may be worth changing the naming to indicate that these are
>>>>>> about the rx cpu (they are, right?). For some applications (sparse,
>>>>>> low-latency flows, for example), it can be useful to keep the tx
>>>>>> completion handling on a different CPU.
>>>>>
>>>>> SO_INCOMING_CPU is rx, like incoming ;)
>>>>>
>>>>>
>>>
>>> Duh :)
>>>
>>>>> --
>>>>> 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
>>>
>>>
>>>
>>> --
>>> Andy Lutomirski
>>> AMA Capital Management, LLC
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] Revert "fast_hash: avoid indirect function calls"
From: David Miller @ 2014-11-14 21:36 UTC (permalink / raw)
To: jay.vosburgh
Cc: hannes, eric.dumazet, netdev, ogerlitz, pshelar, jesse, discuss
In-Reply-To: <16481.1415991906@famine>
From: Jay Vosburgh <jay.vosburgh@canonical.com>
Date: Fri, 14 Nov 2014 11:05:06 -0800
>
> 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>
Applied, thanks Jay.
^ 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