* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
From: Tony Lindgren @ 2016-10-28 15:52 UTC (permalink / raw)
To: Jeroen Hofstee
Cc: netdev, Mugunthan V N, Grygorii Strashko,
open list:TI ETHERNET SWITCH DRIVER (CPSW), open list
In-Reply-To: <1477668756-2651-1-git-send-email-jhofstee@victronenergy.com>
* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> id to common file") did not only move the code for an am3517, it also
> added the slave parameter, resulting in an invalid (all zero) mac address
> being returned for an am3517, since it only has a single emac and the slave
> parameter is pointing to the second. So simply always read the first and
> valid mac-address for a ti,am3517-emac.
And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
From: Florian Westphal @ 2016-10-28 15:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Florian Westphal, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20161028153124.3773180-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> The newly added nft fib code produces two warnings:
>
> net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The first one is obvious as the only user of that variable is
> inside of an #ifdef, but the second one is a bit trickier.
> It is clear that 'oif' is uninitialized here if neither
> NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
>
> I have no idea how that should be handled, this patch just
> returns without doing anything, which may or may not be
> the right thing to do.
It should be initialized to NULL.
^ permalink raw reply
* Re: [Intel-wired-lan] [net-next PATCH 3/3] net: Add support for XPS with QoS via traffic classes
From: John Fastabend @ 2016-10-28 15:49 UTC (permalink / raw)
To: Alexander Duyck, Tom Herbert
Cc: Linux Kernel Network Developers, intel-wired-lan, David S. Miller
In-Reply-To: <CAKgT0UcZY4eKP0ezsijiJquqGKEwap8kA3Kqmg0BRYL8JF_90g@mail.gmail.com>
On 16-10-28 07:58 AM, Alexander Duyck wrote:
> On Thu, Oct 27, 2016 at 7:38 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Thu, Oct 27, 2016 at 8:40 AM, Alexander Duyck
>> <alexander.h.duyck@intel.com> wrote:
>>> This patch adds support for setting and using XPS when QoS via traffic
>>> classes is enabled. With this change we will factor in the priority and
>>> traffic class mapping of the packet and use that information to correctly
>>> select the queue.
>>>
>>> This allows us to define a set of queues for a given traffic class via
>>> mqprio and then configure the XPS mapping for those queues so that the
>>> traffic flows can avoid head-of-line blocking between the individual CPUs
>>> if so desired.
>>>
>> Does this change the sys API for XPS? Is it up the user to know which
>> are priority queues in sys?
>
> The idea was to keep the change transparent. So for now the only
> change in relation to XPS from the XPS point of view is that the map
> for a given queue is invalidated when either the dev->num_tcs changes
> or if the queue is moved into a dev->tx_to_txq mapping. Otherwise the
> interface should behave exactly the same as before.
>
> One thing I could look at doing is adding a read-only sysfs value that
> the user could use to identify which traffic class a given queue
> belongs to. Then at least that way they would be able to dump both
> the XPS map and the tc to determine how the traffic will flow through
> the device.
>
I could see some value in a sysfs read-only tc-queue mapping might be
especially for devices that are negotiating these things using firmware.
.John
^ permalink raw reply
* Re: [net-next PATCH 00/27] Add support for DMA writable pages being writable by the network stack
From: Alexander Duyck @ 2016-10-28 15:48 UTC (permalink / raw)
To: David Miller
Cc: Netdev, intel-wired-lan, linux-kernel@vger.kernel.org, linux-mm,
Jesper Dangaard Brouer, Alexander Duyck, Konrad Rzeszutek Wilk,
Jeff Kirsher
In-Reply-To: <20161025153220.4815.61239.stgit@ahduyck-blue-test.jf.intel.com>
On Tue, Oct 25, 2016 at 8:36 AM, Alexander Duyck
<alexander.h.duyck@intel.com> wrote:
> The first 22 patches in the set add support for the DMA attribute
> DMA_ATTR_SKIP_CPU_SYNC on multiple platforms/architectures. This is needed
> so that we can flag the calls to dma_map/unmap_page so that we do not
> invalidate cache lines that do not currently belong to the device. Instead
> we have to take care of this in the driver via a call to
> sync_single_range_for_cpu prior to freeing the Rx page.
>
> Patch 23 adds support for dma_map_page_attrs and dma_unmap_page_attrs so
> that we can unmap and map a page using the DMA_ATTR_SKIP_CPU_SYNC
> attribute.
>
> Patch 24 adds support for freeing a page that has multiple references being
> held by a single caller. This way we can free page fragments that were
> allocated by a given driver.
>
> The last 3 patches use these updates in the igb driver to allow for us to
> reimpelement the use of build_skb.
>
> My hope is to get the series accepted into the net-next tree as I have a
> number of other Intel drivers I could then begin updating once these
> patches are accepted.
>
> v1: Split out changes DMA_ERROR_CODE fix for swiotlb-xen
> Minor fixes based on issues found by kernel build bot
> Few minor changes for issues found on code review
> Added Acked-by for patches that were acked and not changed
So the feedback for this set has been mostly just a few "Acked-by"s,
and it looks like the series was marked as "Not Applicable" in
patchwork. I was wondering what the correct merge strategy for this
patch set should be going forward?
I was wondering if I should be looking at breaking up the set and
splitting it over a few different trees, or if I should just hold onto
it and resubmit it when the merge window opens? My preference would
be to submit it as a single set so I can know all the patches are
present to avoid any possible regressions due to only part of the set
being present.
Anyway, I am just trying to figure out how best to proceed from here
since these patch sets that touch multiple areas are always
complicated to get submitted.
Thanks.
- Alex
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
From: Johannes Berg @ 2016-10-28 15:46 UTC (permalink / raw)
To: Filip Matusiak, linux-wireless-u79uwXL29TY76Z2rM5mHXA
Cc: marek.kwaczynski-++hxYGjEMp0AvxtiuMwx3w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477665523-8970-1-git-send-email-filip.matusiak-++hxYGjEMp0AvxtiuMwx3w@public.gmane.org>
On Fri, 2016-10-28 at 16:38 +0200, Filip Matusiak wrote:
> This is a workaround for VHT-enabled STAs which break the spec
> and have the VHT-MCS Rx map filled in with value 3 for all eight
> spacial streams.
>
> As per spec, in section 22.1.1 Introduction to the VHT PHY
> A VHT STA shall support at least single spactial stream VHT-MCSs
> 0 to 7 (transmit and receive) in all supported channel widths.
Interesting, and also kinda dumb :)
> + /*
> + * This is a workaround for VHT-enabled STAs which break the
>
I find that it helps, in the future, if we know an example of a station
that did this, so we can test other implementations etc. Can you change
the comment accordingly?
johannes
^ permalink raw reply
* Re: [PATCH] net: smsc911x: Synchronize the runtime PM status during system suspend
From: Jeremy Linton @ 2016-10-28 15:33 UTC (permalink / raw)
To: Ulf Hansson, Rafael J. Wysocki, Alan Stern, linux-pm, netdev
Cc: Len Brown, Pavel Machek, Kevin Hilman, Geert Uytterhoeven,
Lina Iyer, Jon Hunter, Marek Szyprowski, Linus Walleij,
Steve Glendinning
In-Reply-To: <1477567434-5128-1-git-send-email-ulf.hansson@linaro.org>
Hi,
On 10/27/2016 06:23 AM, Ulf Hansson wrote:
> The smsc911c driver puts its device into low power state when entering
> system suspend. Although it doesn't update the device's runtime PM status
> to RPM_SUSPENDED, which causes problems for a parent device.
>
> In particular, when the runtime PM status of the parent is requested to be
> updated to RPM_SUSPENDED, the runtime PM core prevent this, because it's
> forbidden to runtime suspend a device, which has an active child.
>
> Fix this by updating the runtime PM status of the smsc911x device to
> RPM_SUSPENDED during system suspend. In system resume, let's reverse that
> action by runtime resuming the device and thus also the parent.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Steve Glendinning <steve.glendinning@shawell.net>
> Fixes: 8b1107b85efd ("PM / Runtime: Don't allow to suspend a device with an active child")
> ---
>
> Note that the commit this change fixes is currently queued for 4.10 via
> Rafael's linux-pm tree. So this fix should go via that tree as well.
>
> ---
> drivers/net/ethernet/smsc/smsc911x.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index e9b8579..65fca9c 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2584,6 +2584,9 @@ static int smsc911x_suspend(struct device *dev)
> PMT_CTRL_PM_MODE_D1_ | PMT_CTRL_WOL_EN_ |
> PMT_CTRL_ED_EN_ | PMT_CTRL_PME_EN_);
>
> + pm_runtime_disable(dev);
> + pm_runtime_set_suspended(dev);
> +
> return 0;
> }
>
> @@ -2593,6 +2596,9 @@ static int smsc911x_resume(struct device *dev)
> struct smsc911x_data *pdata = netdev_priv(ndev);
> unsigned int to = 100;
>
> + pm_runtime_enable(dev);
> + pm_runtime_resume(dev);
> +
> /* Note 3.11 from the datasheet:
> * "When the LAN9220 is in a power saving state, a write of any
> * data to the BYTE_TEST register will wake-up the device."
>
This seems an unusual change/sequence. I thought a successful return
from the suspend callback would set the device state to suspended.
I just checked a few other ethernet drivers suspend/resume sequences and
directly calling the pm_runtime seems a little unusual. Most of the
other drivers are checking to see if the interface is running then doing
a netif_device_detach()/attach() sequence which is missing from this
drivers suspend/resume path. Could that be part of the problem?
Of course my knowledge of the power management system is a little thin
so I could be really off base.
^ permalink raw reply
* [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
From: Arnd Bergmann @ 2016-10-28 15:31 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Arnd Bergmann, Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Florian Westphal, netfilter-devel, coreteam, netdev, linux-kernel
The newly added nft fib code produces two warnings:
net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
The first one is obvious as the only user of that variable is
inside of an #ifdef, but the second one is a bit trickier.
It is clear that 'oif' is uninitialized here if neither
NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
I have no idea how that should be handled, this patch just
returns without doing anything, which may or may not be
the right thing to do.
Fixes: 84f5eedb983e ("netfilter: nf_tables: add fib expression")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv4/netfilter/nft_fib_ipv4.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
index 6787c563cfc9..b29f70593e8b 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -77,7 +77,9 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
};
const struct net_device *oif;
struct net_device *found;
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
int i;
+#endif
/*
* Do not set flowi4_oif, it restricts results (for example, asking
@@ -90,6 +92,8 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
oif = pkt->out;
else if (priv->flags & NFTA_FIB_F_IIF)
oif = pkt->in;
+ else
+ return;
if (pkt->hook == NF_INET_PRE_ROUTING && fib4_is_local(pkt->skb)) {
nft_fib_store_result(dest, priv->result, pkt, LOOPBACK_IFINDEX);
--
2.9.0
^ permalink raw reply related
* Re: [PATCH net-next 1/1] net: phy: Add support for Microsemi VSC 8530/40 Fast Ethernet PHY
From: Andrew Lunn @ 2016-10-28 15:27 UTC (permalink / raw)
To: Allan W. Nielsen; +Cc: netdev, raju.lakkaraju
In-Reply-To: <1477649411-31036-2-git-send-email-allan.nielsen@microsemi.com>
On Fri, Oct 28, 2016 at 12:10:11PM +0200, Allan W. Nielsen wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: David Ahern @ 2016-10-28 15:00 UTC (permalink / raw)
To: Pablo Neira Ayuso, Alexei Starovoitov
Cc: Daniel Mack, htejun, daniel, ast, davem, kafai, fw, harald,
netdev, sargun, cgroups
In-Reply-To: <20161028112839.GA29798@salvia>
On 10/28/16 5:28 AM, Pablo Neira Ayuso wrote:
> I saw those, I would really like to have a closer look at David
> Ahern's usecase since that skb iif mangling looks kludgy to me, and
> given this is exposing a new helper for general use, not only vrf, it
> would be good to make sure helpers provide something useful for
> everyone. So that new helper is questionable at this stage IMO. I'm
> concerned that people may start using bpf as the adhesive tape to glue
> things to solve probably design problems.
It's changing sk_bound_dev_if on a socket when the socket is created, not skb iif. I explain the need in the cover letter. I have also explained the use case in the other attempts to solve this problem -- a stand alone l3mdev cgroup and adding a default setting to the task struct.
^ permalink raw reply
* Re: [net-next PATCH RFC 07/26] arch/c6x: Add option to skip sync on DMA map and unmap
From: Mark Salter @ 2016-10-28 14:59 UTC (permalink / raw)
To: Alexander Duyck, netdev, linux-kernel, linux-mm
Cc: brouer, davem, linux-c6x-dev, Aurelien Jacquiot
In-Reply-To: <20161024120503.16276.44357.stgit@ahduyck-blue-test.jf.intel.com>
On Mon, 2016-10-24 at 08:05 -0400, Alexander Duyck wrote:
> This change allows us to pass DMA_ATTR_SKIP_CPU_SYNC which allows us to
> avoid invoking cache line invalidation if the driver will just handle it
> later via a sync_for_cpu or sync_for_device call.
>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
> Cc: linux-c6x-dev@linux-c6x.org
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
Acked-by: Mark Salter <msalter@redhat.com>
> arch/c6x/kernel/dma.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/c6x/kernel/dma.c b/arch/c6x/kernel/dma.c
> index db4a6a3..d28df74 100644
> --- a/arch/c6x/kernel/dma.c
> +++ b/arch/c6x/kernel/dma.c
> @@ -42,14 +42,17 @@ static dma_addr_t c6x_dma_map_page(struct device *dev, struct page *page,
> {
> dma_addr_t handle = virt_to_phys(page_address(page) + offset);
>
> - c6x_dma_sync(handle, size, dir);
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + c6x_dma_sync(handle, size, dir);
> +
> return handle;
> }
>
> static void c6x_dma_unmap_page(struct device *dev, dma_addr_t handle,
> size_t size, enum dma_data_direction dir, unsigned long attrs)
> {
> - c6x_dma_sync(handle, size, dir);
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + c6x_dma_sync(handle, size, dir);
> }
>
> static int c6x_dma_map_sg(struct device *dev, struct scatterlist *sglist,
> @@ -60,7 +63,8 @@ static int c6x_dma_map_sg(struct device *dev, struct scatterlist *sglist,
>
> for_each_sg(sglist, sg, nents, i) {
> sg->dma_address = sg_phys(sg);
> - c6x_dma_sync(sg->dma_address, sg->length, dir);
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + c6x_dma_sync(sg->dma_address, sg->length, dir);
> }
>
> return nents;
> @@ -72,8 +76,10 @@ static void c6x_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
> struct scatterlist *sg;
> int i;
>
> - for_each_sg(sglist, sg, nents, i)
> - c6x_dma_sync(sg_dma_address(sg), sg->length, dir);
> + for_each_sg(sglist, sg, nents, i) {
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + c6x_dma_sync(sg_dma_address(sg), sg->length, dir);
> + }
>
> }
>
>
^ permalink raw reply
* Re: [Intel-wired-lan] [net-next PATCH 3/3] net: Add support for XPS with QoS via traffic classes
From: Alexander Duyck @ 2016-10-28 14:58 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers, intel-wired-lan,
David S. Miller
In-Reply-To: <CALx6S36F=6R12O6Xb-WfhruOQN7S6QjOite7PVW2JqNK5BtWYg@mail.gmail.com>
On Thu, Oct 27, 2016 at 7:38 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, Oct 27, 2016 at 8:40 AM, Alexander Duyck
> <alexander.h.duyck@intel.com> wrote:
>> This patch adds support for setting and using XPS when QoS via traffic
>> classes is enabled. With this change we will factor in the priority and
>> traffic class mapping of the packet and use that information to correctly
>> select the queue.
>>
>> This allows us to define a set of queues for a given traffic class via
>> mqprio and then configure the XPS mapping for those queues so that the
>> traffic flows can avoid head-of-line blocking between the individual CPUs
>> if so desired.
>>
> Does this change the sys API for XPS? Is it up the user to know which
> are priority queues in sys?
The idea was to keep the change transparent. So for now the only
change in relation to XPS from the XPS point of view is that the map
for a given queue is invalidated when either the dev->num_tcs changes
or if the queue is moved into a dev->tx_to_txq mapping. Otherwise the
interface should behave exactly the same as before.
One thing I could look at doing is adding a read-only sysfs value that
the user could use to identify which traffic class a given queue
belongs to. Then at least that way they would be able to dump both
the XPS map and the tc to determine how the traffic will flow through
the device.
Thanks.
- Alex
^ permalink raw reply
* Re: [Intel-wired-lan] [net-next PATCH 2/3] net: Refactor removal of queues from XPS map and apply on num_tc changes
From: Alexander Duyck @ 2016-10-28 14:54 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers, intel-wired-lan,
David S. Miller
In-Reply-To: <CALx6S36OgqbtkUp76Mf23C7Xn7EnqPN_Ybyjz-60jn7gE=94pg@mail.gmail.com>
On Thu, Oct 27, 2016 at 7:35 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, Oct 27, 2016 at 8:40 AM, Alexander Duyck
> <alexander.h.duyck@intel.com> wrote:
>> This patch updates the code for removing queues from the XPS map and makes
>> it so that we can apply the code any time we change either the number of
>> traffic classes or the mapping of a given block of queues. This way we
>> avoid having queues pulling traffic from a foreign traffic class.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>> net/core/dev.c | 79 ++++++++++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 56 insertions(+), 23 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index d4d45bf..d124081 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1953,32 +1953,56 @@ static void netif_setup_tc(struct net_device *dev, unsigned int txq)
>> #define xmap_dereference(P) \
>> rcu_dereference_protected((P), lockdep_is_held(&xps_map_mutex))
>>
>> -static struct xps_map *remove_xps_queue(struct xps_dev_maps *dev_maps,
>> - int cpu, u16 index)
>> +static bool remove_xps_queue(struct xps_dev_maps *dev_maps,
>> + int tci, u16 index)
>> {
>> struct xps_map *map = NULL;
>> int pos;
>>
>> if (dev_maps)
>> - map = xmap_dereference(dev_maps->cpu_map[cpu]);
>> + map = xmap_dereference(dev_maps->cpu_map[tci]);
>> + if (!map)
>> + return false;
>>
>> - for (pos = 0; map && pos < map->len; pos++) {
>> - if (map->queues[pos] == index) {
>> - if (map->len > 1) {
>> - map->queues[pos] = map->queues[--map->len];
>> - } else {
>> - RCU_INIT_POINTER(dev_maps->cpu_map[cpu], NULL);
>> - kfree_rcu(map, rcu);
>> - map = NULL;
>> - }
>> + for (pos = map->len; pos--;) {
>> + if (map->queues[pos] != index)
>> + continue;
>> +
>> + if (map->len > 1) {
>> + map->queues[pos] = map->queues[--map->len];
>> break;
>> }
>> +
>> + RCU_INIT_POINTER(dev_maps->cpu_map[tci], NULL);
>> + kfree_rcu(map, rcu);
>> + return false;
>> }
>>
>> - return map;
>> + return true;
>> }
>>
>> -static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
>> +static bool remove_xps_queue_cpu(struct net_device *dev,
>> + struct xps_dev_maps *dev_maps,
>> + int cpu, u16 offset, u16 count)
>> +{
>> + bool active = false;
>> + int i;
>> +
>> + count += offset;
>> + i = count;
>> +
>> + do {
>> + if (i-- == offset) {
>> + active = true;
>> + break;
>> + }
>> + } while (remove_xps_queue(dev_maps, cpu, i));
>> +
> IMO do/while's are hard to read. Does something like this work:
>
> static bool remove_xps_queue_cpu(struct net_device *dev,
> struct xps_dev_maps *dev_maps,
> int cpu, u16 offset, u16 count)
> {
> int i;
>
> for (i = count + offset; i > offset; i--)
> if (!remove_xps_queue(dev_maps, cpu, i - 1))
> break;
>
> return (i == offset);
> }
I can flip the do/while back to a for loop. That shouldn't be too big
of a deal, although I might see if I could convert the loop to do a
pre-decrement instead of a post-decrement. Then I could just check
for (i - offset) < 0.
>> + return active;
>> +}
>> +
>> +static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
>> + u16 count)
>> {
>> struct xps_dev_maps *dev_maps;
>> int cpu, i;
>> @@ -1990,21 +2014,16 @@ static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
>> if (!dev_maps)
>> goto out_no_maps;
>>
>> - for_each_possible_cpu(cpu) {
>> - for (i = index; i < dev->num_tx_queues; i++) {
>> - if (!remove_xps_queue(dev_maps, cpu, i))
>> - break;
>> - }
>> - if (i == dev->num_tx_queues)
>> - active = true;
>> - }
>> + for_each_possible_cpu(cpu)
>> + active |= remove_xps_queue_cpu(dev, dev_maps, cpu, offset,
>> + count);
>
> Maybe just do dumb "if (remove_xps...) active = true;"
I prefer the |= for a loop where the initial value is false and we are
looping and setting it to true on a given condition, especially when
the given condition is a Boolean value. It results in faster and
smaller code using the |= since it is literally just an OR operation
instead of having to do a TEST/JMP/MOV combination.
^ permalink raw reply
* [PATCH] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
From: Filip Matusiak @ 2016-10-28 14:38 UTC (permalink / raw)
To: linux-wireless
Cc: filip.matusiak, marek.kwaczynski, johannes, davem, netdev,
linux-kernel
This is a workaround for VHT-enabled STAs which break the spec
and have the VHT-MCS Rx map filled in with value 3 for all eight
spacial streams.
As per spec, in section 22.1.1 Introduction to the VHT PHY
A VHT STA shall support at least single spactial stream VHT-MCSs
0 to 7 (transmit and receive) in all supported channel widths.
For some devices firmware asserts if such situation occurs.
Signed-off-by: Filip Matusiak <filip.matusiak@tieto.com>
---
net/mac80211/vht.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ee71576..ce93cff 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -270,6 +270,22 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
vht_cap->vht_mcs.tx_mcs_map |= cpu_to_le16(peer_tx << i * 2);
}
+ /*
+ * This is a workaround for VHT-enabled STAs which break the spec
+ * and have the VHT-MCS Rx map filled in with value 3 for all eight
+ * spacial streams.
+ *
+ * As per spec, in section 22.1.1 Introduction to the VHT PHY
+ * A VHT STA shall support at least single spactial stream VHT-MCSs
+ * 0 to 7 (transmit and receive) in all supported channel widths.
+ */
+ if (vht_cap->vht_mcs.rx_mcs_map == cpu_to_le16(0xFFFF)) {
+ vht_cap->vht_supported = false;
+ sdata_info(sdata, "Ignoring VHT IE from %pM due to invalid rx_mcs_map\n",
+ sta->addr);
+ return;
+ }
+
/* finally set up the bandwidth */
switch (vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
case IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net 2/2] geneve: avoid using stale geneve socket.
From: John W. Linville @ 2016-10-28 14:27 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
In-Reply-To: <1477594316-118234-2-git-send-email-pshelar@ovn.org>
On Thu, Oct 27, 2016 at 11:51:56AM -0700, Pravin B Shelar wrote:
> This patch is similar to earlier vxlan patch.
> Geneve device close operation frees geneve socket. This
> operation can race with geneve-xmit function which
> dereferences geneve socket. Following patch uses RCU
> mechanism to avoid this situation.
>
> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
LGTM, although I reckon that Stephen's comment about RCU_INIT_POINTER
applies to this patch as well.
Either way...
Acked-by: John W. Linville <linville@tuxdriver.com>
> ---
> drivers/net/geneve.c | 45 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 16af1ce..42edd7b 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -58,9 +58,9 @@ struct geneve_dev {
> struct hlist_node hlist; /* vni hash table */
> struct net *net; /* netns for packet i/o */
> struct net_device *dev; /* netdev for geneve tunnel */
> - struct geneve_sock *sock4; /* IPv4 socket used for geneve tunnel */
> + struct geneve_sock __rcu *sock4; /* IPv4 socket used for geneve tunnel */
> #if IS_ENABLED(CONFIG_IPV6)
> - struct geneve_sock *sock6; /* IPv6 socket used for geneve tunnel */
> + struct geneve_sock __rcu *sock6; /* IPv6 socket used for geneve tunnel */
> #endif
> u8 vni[3]; /* virtual network ID for tunnel */
> u8 ttl; /* TTL override */
> @@ -543,9 +543,19 @@ static void __geneve_sock_release(struct geneve_sock *gs)
>
> static void geneve_sock_release(struct geneve_dev *geneve)
> {
> - __geneve_sock_release(geneve->sock4);
> + struct geneve_sock *gs4 = rtnl_dereference(geneve->sock4);
> #if IS_ENABLED(CONFIG_IPV6)
> - __geneve_sock_release(geneve->sock6);
> + struct geneve_sock *gs6 = rtnl_dereference(geneve->sock6);
> +
> + rcu_assign_pointer(geneve->sock6, NULL);
> +#endif
> +
> + rcu_assign_pointer(geneve->sock4, NULL);
> + synchronize_net();
> +
> + __geneve_sock_release(gs4);
> +#if IS_ENABLED(CONFIG_IPV6)
> + __geneve_sock_release(gs6);
> #endif
> }
>
> @@ -586,10 +596,10 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
> gs->flags = geneve->flags;
> #if IS_ENABLED(CONFIG_IPV6)
> if (ipv6)
> - geneve->sock6 = gs;
> + rcu_assign_pointer(geneve->sock6, gs);
> else
> #endif
> - geneve->sock4 = gs;
> + rcu_assign_pointer(geneve->sock4, gs);
>
> hash = geneve_net_vni_hash(geneve->vni);
> hlist_add_head_rcu(&geneve->hlist, &gs->vni_list[hash]);
> @@ -603,9 +613,7 @@ static int geneve_open(struct net_device *dev)
> bool metadata = geneve->collect_md;
> int ret = 0;
>
> - geneve->sock4 = NULL;
> #if IS_ENABLED(CONFIG_IPV6)
> - geneve->sock6 = NULL;
> if (ipv6 || metadata)
> ret = geneve_sock_add(geneve, true);
> #endif
> @@ -720,6 +728,9 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
> struct rtable *rt = NULL;
> __u8 tos;
>
> + if (!rcu_dereference(geneve->sock4))
> + return ERR_PTR(-EIO);
> +
> memset(fl4, 0, sizeof(*fl4));
> fl4->flowi4_mark = skb->mark;
> fl4->flowi4_proto = IPPROTO_UDP;
> @@ -772,11 +783,15 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
> {
> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs6 = geneve->sock6;
> struct dst_entry *dst = NULL;
> struct dst_cache *dst_cache;
> + struct geneve_sock *gs6;
> __u8 prio;
>
> + gs6 = rcu_dereference(geneve->sock6);
> + if (!gs6)
> + return ERR_PTR(-EIO);
> +
> memset(fl6, 0, sizeof(*fl6));
> fl6->flowi6_mark = skb->mark;
> fl6->flowi6_proto = IPPROTO_UDP;
> @@ -842,7 +857,7 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> struct ip_tunnel_info *info)
> {
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs4 = geneve->sock4;
> + struct geneve_sock *gs4;
> struct rtable *rt = NULL;
> const struct iphdr *iip; /* interior IP header */
> int err = -EINVAL;
> @@ -853,6 +868,10 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
> u32 flags = geneve->flags;
>
> + gs4 = rcu_dereference(geneve->sock4);
> + if (!gs4)
> + goto tx_error;
> +
> if (geneve->collect_md) {
> if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
> netdev_dbg(dev, "no tunnel metadata\n");
> @@ -932,9 +951,9 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> struct ip_tunnel_info *info)
> {
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs6 = geneve->sock6;
> struct dst_entry *dst = NULL;
> const struct iphdr *iip; /* interior IP header */
> + struct geneve_sock *gs6;
> int err = -EINVAL;
> struct flowi6 fl6;
> __u8 prio, ttl;
> @@ -943,6 +962,10 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
> u32 flags = geneve->flags;
>
> + gs6 = rcu_dereference(geneve->sock6);
> + if (!gs6)
> + goto tx_error;
> +
> if (geneve->collect_md) {
> if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
> netdev_dbg(dev, "no tunnel metadata\n");
> --
> 1.9.1
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
From: Tom Herbert @ 2016-10-28 14:25 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: Linux Kernel Network Developers, LKML, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy
In-Reply-To: <87a8dox4a7.fsf@redhat.com>
On Fri, Oct 28, 2016 at 1:32 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
> On Thu, Oct 27, 2016 at 10:35 PM GMT, Tom Herbert wrote:
>> On Mon, Oct 24, 2016 at 2:28 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
>>> Same as for the transmit path, let's do our best to ensure that received
>>> ICMP errors that may be subject to forwarding will be routed the same
>>> path as flow that triggered the error, if it was going in the opposite
>>> direction.
>>>
>> Unfortunately our ability to do this is generally quite limited. This
>> patch will select the route for multipath, but I don't believe sets
>> the same link in LAG and definitely can't help switches doing ECMP to
>> route the ICMP packet in the same way as the flow would be. Did you
>> see a problem that warrants solving this case?
>
> The motivation here is to bring IPv6 ECMP routing on par with IPv4 to
> enable its wider use, targeting anycast services. Forwarding ICMP errors
> back to the source host, at the L3 layer, is what we thought would be a
> step forward.
>
> Similar to change in IPv4 routing introduced in commit 79a131592dbb
> ("ipv4: ICMP packet inspection for multipath", [1]) we do our best at
> L3, leaving any potential problems with LAG at lower layer (L2)
> unaddressed.
>
ICMP will almost certainly take a different path in the network than
TCP or UDP due to ECMP. If we ever get proper flow label support for
ECMP then that could solve the problem if all the devices do a hash
just on <srcIP, destIP, FlowLabel>.
If this patch is being done to be compatible with IPv4 I guess that's
okay, but it would be false advertisement to say this makes ICMP
follow the same path as the flow being targeted in an error.
Fortunately, I doubt anyone can have a dependency on this for ICMP.
In the realm of OAM with UDP encapsulation this requirement does come
up (that OAM messages can follow the same path as a particular flow).
That case is solvable by always using a UDP encapsulation with same
addresses, ports, and flow label. Unfortunately for that we still have
a few devices that insist on looking into the UDP payload to do
ECMP...
Tom
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 1184c2b..c0f38ea 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>
> [...]
>
>>> @@ -1168,6 +1192,8 @@ void ip6_route_input(struct sk_buff *skb)
>>> tun_info = skb_tunnel_info(skb);
>>> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
>>> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
>>> + if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
>>> + fl6.mp_hash = ip6_multipath_icmp_hash(skb);
>>
>> I will point out that this is only
>
> Sorry, looks like part of your reply got cut short. Could you repost?
>
> -Jakub
>
> [1] https://git.kernel.org/torvalds/c/79a131592dbb81a2dba208622a2ffbfc53f28bc0
^ permalink raw reply
* Re: [PATCH net 3/3] sctp: hold transport instead of assoc when lookup assoc in rx path
From: Neil Horman @ 2016-10-28 14:13 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, daniel
In-Reply-To: <e97fc745a330a6be78b13e1f499381f9d1647b40.1477649076.git.lucien.xin@gmail.com>
On Fri, Oct 28, 2016 at 06:10:54PM +0800, Xin Long wrote:
> Prior to this patch, in rx path, before calling lock_sock, it needed to
> hold assoc when got it by __sctp_lookup_association, in case other place
> would free/put assoc.
>
> But in __sctp_lookup_association, it lookup and hold transport, then got
> assoc by transport->assoc, then hold assoc and put transport. It means
> it didn't hold transport, yet it was returned and later on directly
> assigned to chunk->transport.
>
> Without the protection of sock lock, the transport may be freed/put by
> other places, which would cause a use-after-free issue.
>
> This patch is to fix this issue by holding transport instead of assoc.
> As holding transport can make sure to access assoc is also safe, and
> actually it looks up assoc by searching transport rhashtable, to hold
> transport here makes more sense.
>
> Note that the function will be renamed later on on another patch.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> include/net/sctp/sctp.h | 2 +-
> net/sctp/input.c | 32 ++++++++++++++++----------------
> net/sctp/ipv6.c | 2 +-
> 3 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 87a7f42..31acc3f 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -152,7 +152,7 @@ void sctp_unhash_endpoint(struct sctp_endpoint *);
> struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *,
> struct sctphdr *, struct sctp_association **,
> struct sctp_transport **);
> -void sctp_err_finish(struct sock *, struct sctp_association *);
> +void sctp_err_finish(struct sock *, struct sctp_transport *);
> void sctp_icmp_frag_needed(struct sock *, struct sctp_association *,
> struct sctp_transport *t, __u32 pmtu);
> void sctp_icmp_redirect(struct sock *, struct sctp_transport *,
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 8e0bc58..a01a56e 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -181,9 +181,10 @@ int sctp_rcv(struct sk_buff *skb)
> * bound to another interface, via SO_BINDTODEVICE, treat it as OOTB
> */
> if (sk->sk_bound_dev_if && (sk->sk_bound_dev_if != af->skb_iif(skb))) {
> - if (asoc) {
> - sctp_association_put(asoc);
> + if (transport) {
> + sctp_transport_put(transport);
> asoc = NULL;
> + transport = NULL;
> } else {
> sctp_endpoint_put(ep);
> ep = NULL;
> @@ -269,8 +270,8 @@ int sctp_rcv(struct sk_buff *skb)
> bh_unlock_sock(sk);
>
> /* Release the asoc/ep ref we took in the lookup calls. */
> - if (asoc)
> - sctp_association_put(asoc);
> + if (transport)
> + sctp_transport_put(transport);
> else
> sctp_endpoint_put(ep);
>
> @@ -283,8 +284,8 @@ int sctp_rcv(struct sk_buff *skb)
>
> discard_release:
> /* Release the asoc/ep ref we took in the lookup calls. */
> - if (asoc)
> - sctp_association_put(asoc);
> + if (transport)
> + sctp_transport_put(transport);
> else
> sctp_endpoint_put(ep);
>
> @@ -300,6 +301,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> {
> struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
> struct sctp_inq *inqueue = &chunk->rcvr->inqueue;
> + struct sctp_transport *t = chunk->transport;
> struct sctp_ep_common *rcvr = NULL;
> int backloged = 0;
>
> @@ -351,7 +353,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> done:
> /* Release the refs we took in sctp_add_backlog */
> if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
> - sctp_association_put(sctp_assoc(rcvr));
> + sctp_transport_put(t);
> else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
> sctp_endpoint_put(sctp_ep(rcvr));
> else
> @@ -363,6 +365,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> {
> struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
> + struct sctp_transport *t = chunk->transport;
> struct sctp_ep_common *rcvr = chunk->rcvr;
> int ret;
>
> @@ -373,7 +376,7 @@ static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> * from us
> */
> if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
> - sctp_association_hold(sctp_assoc(rcvr));
> + sctp_transport_hold(t);
> else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
> sctp_endpoint_hold(sctp_ep(rcvr));
> else
> @@ -537,15 +540,15 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
> return sk;
>
> out:
> - sctp_association_put(asoc);
> + sctp_transport_put(transport);
> return NULL;
> }
>
> /* Common cleanup code for icmp/icmpv6 error handler. */
> -void sctp_err_finish(struct sock *sk, struct sctp_association *asoc)
> +void sctp_err_finish(struct sock *sk, struct sctp_transport *t)
> {
> bh_unlock_sock(sk);
> - sctp_association_put(asoc);
> + sctp_transport_put(t);
> }
>
> /*
> @@ -641,7 +644,7 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
> }
>
> out_unlock:
> - sctp_err_finish(sk, asoc);
> + sctp_err_finish(sk, transport);
> }
>
> /*
> @@ -952,11 +955,8 @@ static struct sctp_association *__sctp_lookup_association(
> goto out;
>
> asoc = t->asoc;
> - sctp_association_hold(asoc);
> *pt = t;
>
> - sctp_transport_put(t);
> -
> out:
> return asoc;
> }
> @@ -986,7 +986,7 @@ int sctp_has_association(struct net *net,
> struct sctp_transport *transport;
>
> if ((asoc = sctp_lookup_association(net, laddr, paddr, &transport))) {
> - sctp_association_put(asoc);
> + sctp_transport_put(transport);
> return 1;
> }
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index f473779..176af30 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -198,7 +198,7 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> }
>
> out_unlock:
> - sctp_err_finish(sk, asoc);
> + sctp_err_finish(sk, transport);
> out:
> if (likely(idev != NULL))
> in6_dev_put(idev);
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH net 2/3] sctp: return back transport in __sctp_rcv_init_lookup
From: Neil Horman @ 2016-10-28 14:03 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, daniel
In-Reply-To: <5ebf537ea021af594ca1b2538d28a3080d5c3ee7.1477649076.git.lucien.xin@gmail.com>
On Fri, Oct 28, 2016 at 06:10:53PM +0800, Xin Long wrote:
> Prior to this patch, it used a local variable to save the transport that is
> looked up by __sctp_lookup_association(), and didn't return it back. But in
> sctp_rcv, it is used to initialize chunk->transport. So when hitting this
> code, it was initializing chunk->transport with some random stack value
> instead.
>
> This patch is to return the transport back through transport pointer
> that is from __sctp_rcv_lookup_harder().
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/input.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index a2ea1d1..8e0bc58 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -1021,7 +1021,6 @@ static struct sctp_association *__sctp_rcv_init_lookup(struct net *net,
> struct sctphdr *sh = sctp_hdr(skb);
> union sctp_params params;
> sctp_init_chunk_t *init;
> - struct sctp_transport *transport;
> struct sctp_af *af;
>
> /*
> @@ -1052,7 +1051,7 @@ static struct sctp_association *__sctp_rcv_init_lookup(struct net *net,
>
> af->from_addr_param(paddr, params.addr, sh->source, 0);
>
> - asoc = __sctp_lookup_association(net, laddr, paddr, &transport);
> + asoc = __sctp_lookup_association(net, laddr, paddr, transportp);
> if (asoc)
> return asoc;
> }
> --
> 2.1.0
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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 1/3] sctp: hold transport instead of assoc in sctp_diag
From: Neil Horman @ 2016-10-28 14:01 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, daniel
In-Reply-To: <a58ed3a60643e38876a12ed9193e60f7b38cba4d.1477649076.git.lucien.xin@gmail.com>
On Fri, Oct 28, 2016 at 06:10:52PM +0800, Xin Long wrote:
> In sctp_transport_lookup_process(), Commit 1cceda784980 ("sctp: fix
> the issue sctp_diag uses lock_sock in rcu_read_lock") moved cb() out
> of rcu lock, but it put transport and hold assoc instead, and ignore
> that cb() still uses transport. It may cause a use-after-free issue.
>
> This patch is to hold transport instead of assoc there.
>
> Fixes: 1cceda784980 ("sctp: fix the issue sctp_diag uses lock_sock in rcu_read_lock")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/socket.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9fbb6fe..71b75f9 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4480,12 +4480,9 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
> if (!transport || !sctp_transport_hold(transport))
> goto out;
>
> - sctp_association_hold(transport->asoc);
> - sctp_transport_put(transport);
> -
> rcu_read_unlock();
> err = cb(transport, p);
> - sctp_association_put(transport->asoc);
> + sctp_transport_put(transport);
>
> out:
> return err;
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* [PATCH net-next] udp: do fwd memory scheduling on dequeue
From: Paolo Abeni @ 2016-10-28 13:20 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
A new argument is added to __skb_recv_datagram, it allows
the caller to perform protocol specific action on dequeue
under the receive queue spinlock.
The UDP protocol uses such argument to perform fwd memory
reclaiming on dequeue, while protocol memory and rmem updatating
is delayed after the lock release, to keep the time spent
under the lock as low as possible.
The UDP specific skb desctructor is not used anymore, instead
explicit memory reclaiming is performed at close() time and
when skbs are removed from the receive queue.
The in kernel UDP procotol users now need to use an
skb_recv_udp() variant instead of skb_recv_datagram() to
properly perform memory accounting on dequeue.
Overall, this allows acquiring only once the receive queue
lock on dequeue.
Tested using pktgen with random src port, 64 bytes packet,
wire-speed on a 10G link as sender and udp_sink as the receiver,
using an l4 tuple rxhash to stress the contention, and one or more
udp_sink instances with reuseport.
nr sinks vanilla patched
1 440 560
3 2150 2300
6 3650 3800
9 4450 4600
12 6250 6450
Suggested-by: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Acked-by: Hannes Frederic Sowa <hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>
Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/linux/skbuff.h | 4 ++++
include/net/udp.h | 10 ++++++++
net/core/datagram.c | 11 ++++++---
net/ipv4/udp.c | 65 ++++++++++++++++++++++++++++++++++++++++----------
net/ipv6/udp.c | 3 +--
net/rxrpc/input.c | 7 +++---
net/sunrpc/svcsock.c | 2 +-
net/sunrpc/xprtsock.c | 2 +-
net/unix/af_unix.c | 4 ++--
9 files changed, 83 insertions(+), 25 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 601258f..dd171a9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3028,13 +3028,17 @@ static inline void skb_frag_list_init(struct sk_buff *skb)
#define skb_walk_frags(skb, iter) \
for (iter = skb_shinfo(skb)->frag_list; iter; iter = iter->next)
+typedef void (*skb_dequeue_cb_t)(struct sock *sk, struct sk_buff *skb,
+ int flags);
int __skb_wait_for_more_packets(struct sock *sk, int *err, long *timeo_p,
const struct sk_buff *skb);
struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned flags,
+ skb_dequeue_cb_t dequeue_cb,
int *peeked, int *off, int *err,
struct sk_buff **last);
struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
+ skb_dequeue_cb_t dequeue_cb,
int *peeked, int *off, int *err);
struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags, int noblock,
int *err);
diff --git a/include/net/udp.h b/include/net/udp.h
index 18f1e6b..983c861 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -48,6 +48,7 @@ struct udp_skb_cb {
} header;
__u16 cscov;
__u8 partial_cov;
+ int fwd_memory_released;
};
#define UDP_SKB_CB(__skb) ((struct udp_skb_cb *)((__skb)->cb))
@@ -248,6 +249,15 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
/* net/ipv4/udp.c */
void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb);
+struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags, int noblock,
+ int *peeked, int *off, int *err);
+static inline struct sk_buff *skb_recv_udp(struct sock *sk, unsigned int flags,
+ int noblock, int *err)
+{
+ int peeked, off = 0;
+
+ return __skb_recv_udp(sk, flags, noblock, &peeked, &off, err);
+}
void udp_v4_early_demux(struct sk_buff *skb);
int udp_get_port(struct sock *sk, unsigned short snum,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index bfb973a..226548b 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -165,6 +165,7 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
* __skb_try_recv_datagram - Receive a datagram skbuff
* @sk: socket
* @flags: MSG_ flags
+ * @dequeue_cb: invoked under the receive lock on successful dequeue
* @peeked: returns non-zero if this packet has been seen before
* @off: an offset in bytes to peek skb from. Returns an offset
* within an skb where data actually starts
@@ -197,6 +198,7 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
* the standard around please.
*/
struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
+ skb_dequeue_cb_t dequeue_cb,
int *peeked, int *off, int *err,
struct sk_buff **last)
{
@@ -244,6 +246,8 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
} else
__skb_unlink(skb, queue);
+ if (dequeue_cb)
+ dequeue_cb(sk, skb, flags);
spin_unlock_irqrestore(&queue->lock, cpu_flags);
*off = _off;
return skb;
@@ -262,6 +266,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
EXPORT_SYMBOL(__skb_try_recv_datagram);
struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
+ skb_dequeue_cb_t dequeue_cb,
int *peeked, int *off, int *err)
{
struct sk_buff *skb, *last;
@@ -270,8 +275,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
do {
- skb = __skb_try_recv_datagram(sk, flags, peeked, off, err,
- &last);
+ skb = __skb_try_recv_datagram(sk, flags, dequeue_cb, peeked,
+ off, err, &last);
if (skb)
return skb;
@@ -290,7 +295,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned int flags,
int peeked, off = 0;
return __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, err);
+ NULL, &peeked, &off, err);
}
EXPORT_SYMBOL(skb_recv_datagram);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index c833271..2f1a727 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1172,26 +1172,61 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
return ret;
}
+/* fully reclaim rmem/fwd memory allocated for skb */
static void udp_rmem_release(struct sock *sk, int size, int partial)
{
int amt;
atomic_sub(size, &sk->sk_rmem_alloc);
-
- spin_lock_bh(&sk->sk_receive_queue.lock);
sk->sk_forward_alloc += size;
amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
sk->sk_forward_alloc -= amt;
- spin_unlock_bh(&sk->sk_receive_queue.lock);
if (amt)
__sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
}
-static void udp_rmem_free(struct sk_buff *skb)
+/* if we are not peeking the skb, reclaim fwd allocated memory;
+ * rmem and protocol memory updating is delayed outside the lock
+ */
+static void udp_dequeue(struct sock *sk, struct sk_buff *skb, int flags)
+{
+ int amt;
+
+ if (flags & MSG_PEEK)
+ return;
+
+ sk->sk_forward_alloc += skb->truesize;
+ amt = (sk->sk_forward_alloc - 1) & ~(SK_MEM_QUANTUM - 1);
+ sk->sk_forward_alloc -= amt;
+ UDP_SKB_CB(skb)->fwd_memory_released = amt >> SK_MEM_QUANTUM_SHIFT;
+}
+
+/* complete the memory reclaiming started with udp_dequeue */
+static void __udp_rmem_release(struct sock *sk, struct sk_buff *skb, int flags)
+{
+ int amt = UDP_SKB_CB(skb)->fwd_memory_released;
+
+ if (flags & MSG_PEEK)
+ return;
+
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+ if (amt)
+ __sk_mem_reduce_allocated(sk, amt);
+}
+
+struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
+ int noblock, int *peeked, int *off, int *err)
{
- udp_rmem_release(skb->sk, skb->truesize, 1);
+ struct sk_buff *skb;
+
+ skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
+ udp_dequeue, peeked, off, err);
+ if (skb)
+ __udp_rmem_release(sk, skb, flags);
+ return skb;
}
+EXPORT_SYMBOL(__skb_recv_udp);
int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
{
@@ -1230,7 +1265,6 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
/* the skb owner in now the udp socket */
skb->sk = sk;
- skb->destructor = udp_rmem_free;
skb->dev = NULL;
sock_skb_set_dropcount(sk, skb);
@@ -1254,8 +1288,13 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
static void udp_destruct_sock(struct sock *sk)
{
/* reclaim completely the forward allocated memory */
- __skb_queue_purge(&sk->sk_receive_queue);
- udp_rmem_release(sk, 0, 0);
+ struct sk_buff *skb;
+
+ while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+ udp_rmem_release(sk, skb->truesize, 0);
+ kfree_skb(skb);
+ }
+
inet_sock_destruct(sk);
}
@@ -1303,11 +1342,11 @@ static int first_packet_length(struct sock *sk)
atomic_inc(&sk->sk_drops);
__skb_unlink(skb, rcvq);
__skb_queue_tail(&list_kill, skb);
+ udp_rmem_release(sk, skb->truesize, 1);
+ kfree_skb(skb);
}
res = skb ? skb->len : -1;
spin_unlock_bh(&rcvq->lock);
-
- __skb_queue_purge(&list_kill);
return res;
}
@@ -1362,8 +1401,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
try_again:
peeking = off = sk_peek_offset(sk, flags);
- skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, &err);
+ skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
if (!skb)
return err;
@@ -2583,6 +2621,9 @@ void __init udp_init(void)
{
unsigned long limit;
+ BUILD_BUG_ON(sizeof(struct udp_skb_cb) >
+ FIELD_SIZEOF(struct sk_buff, cb));
+
udp_table_init(&udp_table, "UDP");
limit = nr_free_buffer_pages() / 8;
limit = max(limit, 128UL);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 71963b2..273a806 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -343,8 +343,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
try_again:
peeking = off = sk_peek_offset(sk, flags);
- skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, &err);
+ skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
if (!skb)
return err;
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 44fb8d8..4c36112 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1053,7 +1053,7 @@ void rxrpc_data_ready(struct sock *udp_sk)
ASSERT(!irqs_disabled());
- skb = skb_recv_datagram(udp_sk, 0, 1, &ret);
+ skb = skb_recv_udp(udp_sk, 0, 1, &ret);
if (!skb) {
if (ret == -EAGAIN)
return;
@@ -1075,10 +1075,9 @@ void rxrpc_data_ready(struct sock *udp_sk)
__UDP_INC_STATS(&init_net, UDP_MIB_INDATAGRAMS, 0);
- /* The socket buffer we have is owned by UDP, with UDP's data all over
- * it, but we really want our own data there.
+ /* The UDP protocol already released all skb resources;
+ * we are free to add own data there.
*/
- skb_orphan(skb);
sp = rxrpc_skb(skb);
/* dig out the RxRPC connection details */
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index e2a55dc..78da4ae 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -547,7 +547,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
err = kernel_recvmsg(svsk->sk_sock, &msg, NULL,
0, 0, MSG_PEEK | MSG_DONTWAIT);
if (err >= 0)
- skb = skb_recv_datagram(svsk->sk_sk, 0, 1, &err);
+ skb = skb_recv_udp(svsk->sk_sk, 0, 1, &err);
if (skb == NULL) {
if (err != -EAGAIN) {
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 1758665..7178d0a 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1080,7 +1080,7 @@ static void xs_udp_data_receive(struct sock_xprt *transport)
if (sk == NULL)
goto out;
for (;;) {
- skb = skb_recv_datagram(sk, 0, 1, &err);
+ skb = skb_recv_udp(sk, 0, 1, &err);
if (skb != NULL) {
xs_udp_data_read_skb(&transport->xprt, sk, skb);
consume_skb(skb);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 145082e..8762018 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2113,8 +2113,8 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
mutex_lock(&u->iolock);
skip = sk_peek_offset(sk, flags);
- skb = __skb_try_recv_datagram(sk, flags, &peeked, &skip, &err,
- &last);
+ skb = __skb_try_recv_datagram(sk, flags, NULL, &peeked, &skip,
+ &err, &last);
if (skb)
break;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net-next v3 3/9] ipv6: sr: add support for SRH encapsulation and injection with lwtunnels
From: Roopa Prabhu @ 2016-10-28 12:59 UTC (permalink / raw)
To: David Lebrun; +Cc: netdev
In-Reply-To: <1477575239-11228-4-git-send-email-david.lebrun@uclouvain.be>
On 10/27/16, 6:33 AM, David Lebrun wrote:
> This patch creates a new type of interfaceless lightweight tunnel (SEG6),
> enabling the encapsulation and injection of SRH within locally emitted
> packets and forwarded packets.
>
> From a configuration viewpoint, a seg6 tunnel would be configured as follows:
>
> ip -6 ro ad fc00::1/128 encap seg6 mode encap segs fc42::1,fc42::2,fc42::3 dev eth0
>
> Any packet whose destination address is fc00::1 would thus be encapsulated
> within an outer IPv6 header containing the SRH with three segments, and would
> actually be routed to the first segment of the list. If `mode inline' was
> specified instead of `mode encap', then the SRH would be directly inserted
> after the IPv6 header without outer encapsulation.
>
> Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
^ permalink raw reply
* [PATCH net-next v2 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
From: Jakub Sitnicki @ 2016-10-28 12:32 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Hannes Frederic Sowa, Hideaki YOSHIFUJI,
Tom Herbert
In-Reply-To: <1477657955-5157-1-git-send-email-jkbs@redhat.com>
Same as for the transmit path, let's do our best to ensure that received
ICMP errors that may be subject to forwarding will be routed the same
path as flow that triggered the error, if it was going in the opposite
direction.
v1 -> v2:
- style change, put as many arguments as possible on the first line of
a function call, and align consecutive lines to the first argument,
pointed out by David Miller
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/route.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1184c2b..269e30d 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1150,6 +1150,30 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
}
EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
+static u32 ip6_multipath_icmp_hash(const struct sk_buff *skb)
+{
+ const struct icmp6hdr *icmph = icmp6_hdr(skb);
+ const struct ipv6hdr *inner_iph;
+ struct ipv6hdr _inner_iph;
+
+ if (icmph->icmp6_type != ICMPV6_DEST_UNREACH &&
+ icmph->icmp6_type != ICMPV6_PKT_TOOBIG &&
+ icmph->icmp6_type != ICMPV6_TIME_EXCEED &&
+ icmph->icmp6_type != ICMPV6_PARAMPROB)
+ goto standard_hash;
+
+ inner_iph = skb_header_pointer(skb,
+ skb_transport_offset(skb) + sizeof(*icmph),
+ sizeof(_inner_iph), &_inner_iph);
+ if (!inner_iph)
+ goto standard_hash;
+
+ return icmpv6_multipath_hash(inner_iph);
+
+standard_hash:
+ return 0; /* compute it later, if needed */
+}
+
void ip6_route_input(struct sk_buff *skb)
{
const struct ipv6hdr *iph = ipv6_hdr(skb);
@@ -1168,6 +1192,8 @@ void ip6_route_input(struct sk_buff *skb)
tun_info = skb_tunnel_info(skb);
if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
+ if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
+ fl6.mp_hash = ip6_multipath_icmp_hash(skb);
skb_dst_drop(skb);
skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 4/5] ipv6: Compute multipath hash for sent ICMP errors from offending packet
From: Jakub Sitnicki @ 2016-10-28 12:32 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Hannes Frederic Sowa, Hideaki YOSHIFUJI,
Tom Herbert
In-Reply-To: <1477657955-5157-1-git-send-email-jkbs@redhat.com>
Improve debuggability with tools like traceroute and make PMTUD work in
setups that make use of ECMP routing by sending ICMP errors down the
same path as the offending packet would travel, if it was going in the
opposite direction.
There is a caveat, flows in both directions need use the same
label. Otherwise packets from flow in the opposite direction and ICMP
errors will not be routed over the same ECMP link.
Export the function for calculating the multipath hash so that we can
use it also on receive side, when forwarding ICMP errors.
v1 -> v2:
- don't use "extern" in external function declaration in header file,
pointed out by David Miller
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/linux/icmpv6.h | 2 ++
net/ipv6/icmp.c | 21 +++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/include/linux/icmpv6.h b/include/linux/icmpv6.h
index 57086e9..803c241 100644
--- a/include/linux/icmpv6.h
+++ b/include/linux/icmpv6.h
@@ -45,4 +45,6 @@ extern void icmpv6_flow_init(struct sock *sk,
const struct in6_addr *saddr,
const struct in6_addr *daddr,
int oif);
+struct ipv6hdr;
+u32 icmpv6_multipath_hash(const struct ipv6hdr *iph);
#endif
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index bd59c34..ab376b3d1 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -385,6 +385,26 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net,
return ERR_PTR(err);
}
+u32 icmpv6_multipath_hash(const struct ipv6hdr *iph)
+{
+ struct flowi6 fl6;
+
+ /* Calculate the multipath hash from the offending IP datagram that
+ * triggered the ICMP error. The source and destination addresses are
+ * swapped as we do our best to route the ICMP message together with the
+ * flow it belongs to. However, flows in both directions have to have
+ * the same label (e.g. by using flow label reflection) for it to
+ * happen.
+ */
+ memset(&fl6, 0, sizeof(fl6));
+ fl6.daddr = iph->saddr;
+ fl6.saddr = iph->daddr;
+ fl6.flowlabel = ip6_flowinfo(iph);
+ fl6.flowi6_proto = iph->nexthdr;
+
+ return get_hash_from_flowi6(&fl6);
+}
+
/*
* Send an ICMP message in response to a packet in error
*/
@@ -484,6 +504,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
fl6.flowi6_oif = iif;
fl6.fl6_icmp_type = type;
fl6.fl6_icmp_code = code;
+ fl6.mp_hash = icmpv6_multipath_hash(hdr);
security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
sk = icmpv6_xmit_lock(net);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 1/5] ipv6: Fold rt6_info_hash_nhsfn() into its only caller
From: Jakub Sitnicki @ 2016-10-28 12:32 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Hannes Frederic Sowa, Hideaki YOSHIFUJI,
Tom Herbert
In-Reply-To: <1477657955-5157-1-git-send-email-jkbs@redhat.com>
Commit 644d0e656958 ("ipv6 Use get_hash_from_flowi6 for rt6 hash") has
turned rt6_info_hash_nhsfn() into a one-liner, so it no longer makes
sense to keep it around.
Also the accompanying documentation comment has become outdated, so just
remove it altogether.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/route.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bdbc38e..0514b35 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -425,16 +425,6 @@ static bool rt6_check_expired(const struct rt6_info *rt)
return false;
}
-/* Multipath route selection:
- * Hash based function using packet header and flowlabel.
- * Adapted from fib_info_hashfn()
- */
-static int rt6_info_hash_nhsfn(unsigned int candidate_count,
- const struct flowi6 *fl6)
-{
- return get_hash_from_flowi6(fl6) % candidate_count;
-}
-
static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
struct flowi6 *fl6, int oif,
int strict)
@@ -442,7 +432,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
struct rt6_info *sibling, *next_sibling;
int route_choosen;
- route_choosen = rt6_info_hash_nhsfn(match->rt6i_nsiblings + 1, fl6);
+ route_choosen = get_hash_from_flowi6(fl6) % (match->rt6i_nsiblings + 1);
/* Don't change the route, if route_choosen == 0
* (siblings does not include ourself)
*/
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 3/5] ipv6: Use multipath hash from flow info if available
From: Jakub Sitnicki @ 2016-10-28 12:32 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Hannes Frederic Sowa, Hideaki YOSHIFUJI,
Tom Herbert
In-Reply-To: <1477657955-5157-1-git-send-email-jkbs@redhat.com>
Allow our callers to influence the choice of ECMP link by honoring the
hash passed together with the flow info. This will allow for special
treatment of ICMP errors which we would like to route over the same link
as the IP datagram that triggered the error.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/route.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0514b35..1184c2b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -430,9 +430,11 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
int strict)
{
struct rt6_info *sibling, *next_sibling;
+ unsigned int hash;
int route_choosen;
- route_choosen = get_hash_from_flowi6(fl6) % (match->rt6i_nsiblings + 1);
+ hash = fl6->mp_hash ? : get_hash_from_flowi6(fl6);
+ route_choosen = hash % (match->rt6i_nsiblings + 1);
/* Don't change the route, if route_choosen == 0
* (siblings does not include ourself)
*/
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/5] net: Extend struct flowi6 with multipath hash
From: Jakub Sitnicki @ 2016-10-28 12:32 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Hannes Frederic Sowa, Hideaki YOSHIFUJI,
Tom Herbert
In-Reply-To: <1477657955-5157-1-git-send-email-jkbs@redhat.com>
Allow for functions that fill out the IPv6 flow info to also pass a hash
computed over the skb contents. The hash value will drive the multipath
routing decisions.
This is intended for special treatment of ICMPv6 errors, where we would
like to make a routing decision based on the flow identifying the
offending IPv6 datagram that triggered the error, rather than the flow
of the ICMP error itself.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/flow.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/flow.h b/include/net/flow.h
index 035aa77..73ee3aa 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -143,6 +143,7 @@ struct flowi6 {
#define fl6_ipsec_spi uli.spi
#define fl6_mh_type uli.mht.type
#define fl6_gre_key uli.gre_key
+ __u32 mp_hash;
} __attribute__((__aligned__(BITS_PER_LONG/8)));
struct flowidn {
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox