* Re: [PATCHv3] netem: Segment GSO packets on enqueue
From: Eric Dumazet @ 2016-04-29 18:09 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jamal Hadi Salim, David S. Miller, netem
In-Reply-To: <1461951348-3920-1-git-send-email-nhorman@tuxdriver.com>
On Fri, 2016-04-29 at 13:35 -0400, Neil Horman wrote:
> This was recently reported to me, and reproduced on the latest net kernel, when
> attempting to run netperf from a host that had a netem qdisc attached to the
> egress interface:
>
> /*
> * Insert one skb into qdisc.
> * Note: parent depends on return value to account for queue length.
> @@ -407,7 +426,11 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> /* We don't fill cb now as skb_unshare() may invalidate it */
> struct netem_skb_cb *cb;
> struct sk_buff *skb2;
> + struct sk_buff *segs = NULL;
> + unsigned int len = 0, prev_len = qdisc_pkt_len(skb);
> + int nb = 0;
> int count = 1;
> + int rc = NET_XMIT_SUCCESS;
>
> /* Random duplication */
> if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor))
> @@ -453,10 +476,22 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> * do it now in software before we mangle it.
> */
> if (q->corrupt && q->corrupt >= get_crandom(&q->corrupt_cor)) {
> + if (skb_is_gso(skb)) {
> + segs = netem_segment(skb, sch);
> + if (!segs)
> + return NET_XMIT_DROP;
> + } else
> + segs = skb;
> +
> + skb = segs;
> + segs = segs->next;
> +
> if (!(skb = skb_unshare(skb, GFP_ATOMIC)) ||
> (skb->ip_summed == CHECKSUM_PARTIAL &&
> - skb_checksum_help(skb)))
> - return qdisc_drop(skb, sch);
> + skb_checksum_help(skb))) {
> + rc = qdisc_drop(skb, sch);
> + goto finish_segs;
> + }
>
> skb->data[prandom_u32() % skb_headlen(skb)] ^=
> 1<<(prandom_u32() % 8);
> @@ -516,7 +551,26 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> sch->qstats.requeues++;
> }
>
> - return NET_XMIT_SUCCESS;
> +finish_segs:
> + if (segs) {
> + while (segs) {
> + skb2 = segs->next;
> + segs->next = NULL;
> + qdisc_skb_cb(segs)->pkt_len = segs->len;
> + len += segs->len;
Wrong operation if packet is dropped by qdisc_enqueue() ?
I would use
u32 last_len = segs->len;
> + rc = qdisc_enqueue(segs, sch);
> + if (rc != NET_XMIT_SUCCESS) {
> + if (net_xmit_drop_count(rc))
> + qdisc_qstats_drop(sch);
> + } else
} else {
nb++;
len += last_len;
}
> + nb++;
> + segs = skb2;
> + }
> + sch->q.qlen += nb;
> + if (nb > 1)
> + qdisc_tree_reduce_backlog(sch, 1 - nb, prev_len - len);
> + }
> + return rc;
Seems you should return NET_XMIT_SUCCESS instead of status of last
segment ?
> }
Thanks.
^ permalink raw reply
* [PATCH v2 0/3] drivers: net: xgene: fix: Get channel number from device binding
From: Iyappan Subramanian @ 2016-04-29 18:10 UTC (permalink / raw)
To: davem, netdev, devicetree; +Cc: linux-arm-kernel, patches, Iyappan Subramanian
This patch set adds 'channel' property to get ethernet to CPU channel number,
thus decoupling the Linux driver from static resource selection.
v2: Address review comments from v1
- removed irq reference from Linux driver
- added 'channel' property to get ethernet to CPU channel number
v1:
- Initial version
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---
Iyappan Subramanian (3):
drivers: net: xgene: Get channel number from device binding
Documentation: dtb: xgene: Add channel property
dtb: xgene: Add channel property
Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 2 ++
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 1 +
arch/arm64/boot/dts/apm/apm-storm.dtsi | 1 +
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 15 ++++++++++++++-
4 files changed, 18 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply
* [PATCH v2 1/3] drivers: net: xgene: Get channel number from device binding
From: Iyappan Subramanian @ 2016-04-29 18:10 UTC (permalink / raw)
To: davem, netdev, devicetree; +Cc: linux-arm-kernel, patches, Iyappan Subramanian
In-Reply-To: <1461953415-9741-1-git-send-email-isubramanian@apm.com>
This patch gets ethernet to CPU channel (prefetch buffer number) from
the newly added 'channel' property, thus decoupling Linux driver from
resource management.
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 8d4c1ad..409152b 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -973,6 +973,17 @@ static enum xgene_ring_owner xgene_derive_ring_owner(struct xgene_enet_pdata *p)
return owner;
}
+static u8 xgene_start_cpu_bufnum(struct xgene_enet_pdata *pdata)
+{
+ struct device *dev = &pdata->pdev->dev;
+ u32 cpu_bufnum;
+ int ret;
+
+ ret = device_property_read_u32(dev, "channel", &cpu_bufnum);
+
+ return (!ret) ? cpu_bufnum : pdata->cpu_bufnum;
+}
+
static int xgene_enet_create_desc_rings(struct net_device *ndev)
{
struct xgene_enet_pdata *pdata = netdev_priv(ndev);
@@ -981,13 +992,15 @@ static int xgene_enet_create_desc_rings(struct net_device *ndev)
struct xgene_enet_desc_ring *buf_pool = NULL;
enum xgene_ring_owner owner;
dma_addr_t dma_exp_bufs;
- u8 cpu_bufnum = pdata->cpu_bufnum;
+ u8 cpu_bufnum;
u8 eth_bufnum = pdata->eth_bufnum;
u8 bp_bufnum = pdata->bp_bufnum;
u16 ring_num = pdata->ring_num;
u16 ring_id;
int i, ret, size;
+ cpu_bufnum = xgene_start_cpu_bufnum(pdata);
+
for (i = 0; i < pdata->rxq_cnt; i++) {
/* allocate rx descriptor ring */
owner = xgene_derive_ring_owner(pdata);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/3] Documentation: dtb: xgene: Add channel property
From: Iyappan Subramanian @ 2016-04-29 18:10 UTC (permalink / raw)
To: davem, netdev, devicetree; +Cc: linux-arm-kernel, patches, Iyappan Subramanian
In-Reply-To: <1461953415-9741-1-git-send-email-isubramanian@apm.com>
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---
Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
index 078060a..05f705e3 100644
--- a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
+++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
@@ -18,6 +18,8 @@ Required properties for all the ethernet interfaces:
- First is the Rx interrupt. This irq is mandatory.
- Second is the Tx completion interrupt.
This is supported only on SGMII based 1GbE and 10GbE interfaces.
+- channel: Ethernet to CPU, start channel (prefetch buffer) number
+ - Must map to the first irq and irqs must be sequential
- port-id: Port number (0 or 1)
- clocks: Reference to the clock entry.
- local-mac-address: MAC address assigned to this device
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/3] dtb: xgene: Add channel property
From: Iyappan Subramanian @ 2016-04-29 18:10 UTC (permalink / raw)
To: davem, netdev, devicetree; +Cc: linux-arm-kernel, patches, Iyappan Subramanian
In-Reply-To: <1461953415-9741-1-git-send-email-isubramanian@apm.com>
Added 'channel' property, describing ethernet to CPU channel number.
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 1 +
arch/arm64/boot/dts/apm/apm-storm.dtsi | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index a055a5d..ba04877 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -653,6 +653,7 @@
<0 113 4>,
<0 114 4>,
<0 115 4>;
+ channel = <12>;
port-id = <1>;
dma-coherent;
clocks = <&xge1clk 0>;
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index ae4a173..5147d76 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -993,6 +993,7 @@
<0x0 0x65 0x4>,
<0x0 0x66 0x4>,
<0x0 0x67 0x4>;
+ channel = <0>;
dma-coherent;
clocks = <&xge0clk 0>;
/* mac address will be overwritten by the bootloader */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()
From: Marek Vasut @ 2016-04-29 18:11 UTC (permalink / raw)
To: Sergei Shtylyov, netdev
Cc: Arnd Bergmann, David S . Miller, Dinh Nguyen, Florian Fainelli
In-Reply-To: <572381DA.5030607@cogentembedded.com>
On 04/29/2016 05:46 PM, Sergei Shtylyov wrote:
> Hello.
Hi!
> On 04/29/2016 02:45 PM, Marek Vasut wrote:
>
>>> First of all, thank you for the patch!
>>> You beat me to it (and not only me). :-)
>>
>> Heh, hacking at night has it's perks :)
>
> I know, I'm just supposed to work on different things. :-)
>
>>> On 4/29/2016 4:09 AM, Marek Vasut wrote:
>>>
>>>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>>>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
>>>
>>> scripts/checkpatch.pl now enforces certain commit citing style,
>>> yours
>>> doesn't quite match it.
>>
>> Ha, I didn't know that checkpatch can now warn about this too, nice. Is
>> that in next already ? I just tried checkpatch and it doesn't warn
>> about it.
>
> It's been merged long ago. Actually, I've just run this script from
> the current Linus' tree on your patch, here's the results:
Oh, got it now.
> ERROR: Please use git commit description style 'commit <12+ chars of
> sha1> ("<title line>")' - ie: 'commit fatal: bad o ("cc5d109451c0d5b17")'
> #4:
> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>
> WARNING: 'immediatelly' may be misspelled - perhaps 'immediately'?
> #23:
> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>
> total: 1 errors, 1 warnings, 0 checks, 8 lines checked
Ha, this is embarrassing .
>> Anyway, regarding this format, do you want V2 ? Originally, I had the
>> full commit info in the message, but that was just taking space and
>> it is not the commit which is important in the message, so I trimmed
>> it down.
>
> I'll let DaveM decide.
OK
>>>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>>>> all ones.
>>>>
>>>> This causes problem with stmmac driver and likely some other drivers
>>>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>>>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>>>> there is no PHY node specified in the DT for the stmmac block, the
>>>> stmmac
>>>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>>>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>>>> register the MDIO bus and probe for the PHY.
>>>>
>>>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>>>> the addresses on the MDIO bus and calls mdiobus_scan() for each of
>>>> them,
>>>> which invokes get_phy_device(). Before the aforementioned patch, the
>>>> mdiobus_scan() would return NULL if no PHY was found on a given address
>>>> and mdiobus_register() would continue and try the next PHY address.
>>>> Now,
>>>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>>>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>>>> PHY address does not contain PHY.
>>>>
>>>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>>>> error comes around, continue with the next PHY address.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>> Cc: David S. Miller <davem@davemloft.net>
>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>>> ---
>>>> drivers/net/phy/mdio_bus.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> NOTE: I don't quite like this explicit check , but I don't have better
>>>> idea now.
>>>
>>> It's fine. I was going to do just the same :-)
>>
>> OK, I'm glad I'm not alone on this one :)
>>
>>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>>> index 499003ee..388f992 100644
>>>> --- a/drivers/net/phy/mdio_bus.c
>>>> +++ b/drivers/net/phy/mdio_bus.c
>>>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct
>>>> module *owner)
>>>> struct phy_device *phydev;
>>>>
>>>> phydev = mdiobus_scan(bus, i);
>>>> - if (IS_ERR(phydev)) {
>>>> + if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>>>
>>> Parens around the second operand of && are not really needed
>>> though...
>>
>> While I agree, I also prefer to make things obvious when reading the
>> code by adding the parenthesis. It's a matter of taste I think. Just let
>> me know if I should spin V2 without them :)
>
> Again, let DaveM decide. But I don't think the code being patched
> uses parens in such cases... I wouldn't, for sure.
>
>> Thanks for the review!
>
> You've saved me some work (but I still need to analyze the other call
> paths).
If there is something you want to test on the stmmac, let me know.
>>> [...]
>
> MBR, Sergei
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH net] ip_tunnel: fix preempt warning in ip tunnel creation/updating
From: David Miller @ 2016-04-29 18:11 UTC (permalink / raw)
To: pabeni
Cc: netdev, kuznet, tgraf, pshelar, jbenc, hannes, sergei.shtylyov,
tom, eric.dumazet
In-Reply-To: <b21056e2495c485a2084d058d03e1f95480b3510.1461834203.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 28 Apr 2016 11:04:51 +0200
> After the commit e09acddf873b ("ip_tunnel: replace dst_cache with generic
> implementation"), a preemption debug warning is triggered on ip4
> tunnels updating; the dst cache helper needs to be invoked in unpreemptible
> context.
>
> We don't need to load the cache on tunnel update, so this commit fixes
> the warning replacing the load with a dst cache reset, which is
> preempt safe.
>
> Fixes: e09acddf873b ("ip_tunnel: replace dst_cache with generic
> implementation")
>
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Applied, thanks Paolo.
^ permalink raw reply
* Re: [PATCHv3] netem: Segment GSO packets on enqueue
From: Stephen Hemminger @ 2016-04-29 18:19 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, Jamal Hadi Salim, David S. Miller, netem, eric.dumazet
In-Reply-To: <1461951348-3920-1-git-send-email-nhorman@tuxdriver.com>
On Fri, 29 Apr 2016 13:35:48 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:
> This was recently reported to me, and reproduced on the latest net kernel, when
> attempting to run netperf from a host that had a netem qdisc attached to the
> egress interface:
>
> [ 788.073771] ------------[ cut here ]------------
> [ 788.096716] WARNING: at net/core/dev.c:2253 skb_warn_bad_offload+0xcd/0xda()
> [ 788.129521] bnx2: caps=(0x00000001801949b3, 0x0000000000000000) len=2962
> data_len=0 gso_size=1448 gso_type=1 ip_summed=3
> [ 788.182150] Modules linked in: sch_netem kvm_amd kvm crc32_pclmul ipmi_ssif
> ghash_clmulni_intel sp5100_tco amd64_edac_mod aesni_intel lrw gf128mul
> glue_helper ablk_helper edac_mce_amd cryptd pcspkr sg edac_core hpilo ipmi_si
> i2c_piix4 k10temp fam15h_power hpwdt ipmi_msghandler shpchp acpi_power_meter
> pcc_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c
> sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt
> i2c_algo_bit drm_kms_helper ahci ata_generic pata_acpi ttm libahci
> crct10dif_pclmul pata_atiixp tg3 libata crct10dif_common drm crc32c_intel ptp
> serio_raw bnx2 r8169 hpsa pps_core i2c_core mii dm_mirror dm_region_hash dm_log
> dm_mod
> [ 788.465294] CPU: 16 PID: 0 Comm: swapper/16 Tainted: G W
> ------------ 3.10.0-327.el7.x86_64 #1
> [ 788.511521] Hardware name: HP ProLiant DL385p Gen8, BIOS A28 12/17/2012
> [ 788.542260] ffff880437c036b8 f7afc56532a53db9 ffff880437c03670
> ffffffff816351f1
> [ 788.576332] ffff880437c036a8 ffffffff8107b200 ffff880633e74200
> ffff880231674000
> [ 788.611943] 0000000000000001 0000000000000003 0000000000000000
> ffff880437c03710
> [ 788.647241] Call Trace:
> [ 788.658817] <IRQ> [<ffffffff816351f1>] dump_stack+0x19/0x1b
> [ 788.686193] [<ffffffff8107b200>] warn_slowpath_common+0x70/0xb0
> [ 788.713803] [<ffffffff8107b29c>] warn_slowpath_fmt+0x5c/0x80
> [ 788.741314] [<ffffffff812f92f3>] ? ___ratelimit+0x93/0x100
> [ 788.767018] [<ffffffff81637f49>] skb_warn_bad_offload+0xcd/0xda
> [ 788.796117] [<ffffffff8152950c>] skb_checksum_help+0x17c/0x190
> [ 788.823392] [<ffffffffa01463a1>] netem_enqueue+0x741/0x7c0 [sch_netem]
> [ 788.854487] [<ffffffff8152cb58>] dev_queue_xmit+0x2a8/0x570
> [ 788.880870] [<ffffffff8156ae1d>] ip_finish_output+0x53d/0x7d0
> ...
>
> The problem occurs because netem is not prepared to handle GSO packets (as it
> uses skb_checksum_help in its enqueue path, which cannot manipulate these
> frames).
>
> The solution I think is to simply segment the skb in a simmilar fashion to the
> way we do in __dev_queue_xmit (via validate_xmit_skb), with some minor changes.
> When we decide to corrupt an skb, if the frame is GSO, we segment it, corrupt
> the first segment, and enqueue the remaining ones.
>
> tested successfully by myself on the latest net kernel, to whcih this applies
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jamal Hadi Salim <jhs@mojatatu.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: netem@lists.linux-foundation.org
> CC: eric.dumazet@gmail.com
>
This looks like a good idea.
Please cleanup the formatting issues:
This was recently reported to me, and reproduced on the latest net kernel, when
WARNING: 'whcih' may be misspelled - perhaps 'which'?
#93:
tested successfully by myself on the latest net kernel, to whcih this applies
ERROR: "foo* bar" should be "foo *bar"
#130: FILE: net/sched/sch_netem.c:402:
+static struct sk_buff* netem_segment(struct sk_buff *skb, struct Qdisc *sch)
CHECK: braces {} should be used on all arms of this statement
#164: FILE: net/sched/sch_netem.c:479:
+ if (skb_is_gso(skb)) {
[...]
+ } else
[...]
CHECK: braces {} should be used on all arms of this statement
#198: FILE: net/sched/sch_netem.c:562:
+ if (rc != NET_XMIT_SUCCESS) {
[...]
+ } else
[...]
^ permalink raw reply
* Re: [net-next PATCH V4 0/5] samples/bpf: Improve user experience
From: David Miller @ 2016-04-29 18:26 UTC (permalink / raw)
To: brouer
Cc: netdev, linux-kbuild, bblanco, naveen.n.rao, borkmann,
alexei.starovoitov
In-Reply-To: <20160428121949.16807.84828.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 28 Apr 2016 14:20:48 +0200
> It is a steep learning curve getting started with using the eBPF
> examples in samples/bpf/. There are several dependencies, and
> specific versions of these dependencies. Invoking make in the correct
> manor is also slightly obscure.
>
> This patchset cleanup, document and hopefully improves the first time
> user experience with the eBPF samples directory by auto-detecting
> certain scenarios.
>
> V4:
> - Address Naveen's nitpicks
> - Handle/fail if extra args are passed in LLC or CLANG (David Laight)
>
> V3:
> - Add Alexei's ACKs
> - Remove README paragraph about LLVM experimental BPF target
> as it only existed between LLVM version 3.6 to 3.7.
>
> V2:
> - Adjusted recommend minimum versions to 3.7.1
> - Included clang build instructions
> - New patch adding CLANG variable and validation of command
Series applied, thanks Jesper.
^ permalink raw reply
* Re: [PATCH net-next] vxlan: fix initialization with custom link parameters
From: David Miller @ 2016-04-29 19:09 UTC (permalink / raw)
To: jbenc; +Cc: netdev, nicolas.dichtel
In-Reply-To: <7c19200582d6f4272cee914934f428775e405e89.1461854177.git.jbenc@redhat.com>
From: Jiri Benc <jbenc@redhat.com>
Date: Thu, 28 Apr 2016 16:36:30 +0200
> Commit 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate
> function") changed initialization order and as an unintended result, when the
> user specifies additional link parameters (such as IFLA_ADDRESS) while
> creating vxlan interface, those are overwritten by vxlan_ether_setup later.
>
> It's necessary to call ether_setup from withing the ->setup callback. That
> way, the correct parameters are set by rtnl_create_link later. This is done
> also for VXLAN-GPE, as we don't know the interface type yet at that point,
> and changed to the correct interface type later.
>
> Fixes: 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate function")
> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Applied, thanks Jiri.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: stmmac: update MDIO support for GMAC4
From: David Miller @ 2016-04-29 19:14 UTC (permalink / raw)
To: alexandre.torgue; +Cc: netdev, peppe.cavallaro
In-Reply-To: <1461851805-15201-1-git-send-email-alexandre.torgue@st.com>
From: Alexandre TORGUE <alexandre.torgue@st.com>
Date: Thu, 28 Apr 2016 15:56:45 +0200
> On new GMAC4 IP, MAC_MDIO_address register has been updated, and bitmaps
> changed. This patch takes into account those changes.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
Applied, thanks.
^ permalink raw reply
* [PATCH] Bluetooth: Use hci_conn_hash_lookup_le
From: Julia Lawall @ 2016-04-29 19:22 UTC (permalink / raw)
To: Marcel Holtmann
Cc: kernel-janitors, Gustavo Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth, netdev, linux-kernel
Use the function hci_conn_hash_lookup_le that integrates type testing for
looking up LE connections. See the patch 1b51c7b6 that introduced this
function for more details.
The semantic patch that (sort of) fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression hdev,addr;
symbol UNKNOWN;
@@
- hci_conn_hash_lookup_ba(hdev, LE_LINK, addr)
+ hci_conn_hash_lookup_le(hdev, addr, UNKNOWN)
// </smpl>
UNKNOWN has to be replaced by the address type from the usage context. In
the second case, a subsequent test on the address type is removed also.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
This was done by analogy with f5ad4ff and from the explanation in
1b51c7b6. It is not tested, and I don't really know if it is correct.
net/bluetooth/mgmt.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9e4b931..5a61245 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4773,7 +4773,8 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
&cp->addr.bdaddr);
else
- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
+ conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr,
+ cp->addr.type);
if (!conn || conn->state != BT_CONNECTED) {
err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
@@ -5009,13 +5010,10 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
{
struct hci_conn *conn;
- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
+ conn = hci_conn_hash_lookup_le(hdev, addr, type);
if (!conn)
return false;
- if (conn->dst_type != type)
- return false;
-
if (conn->state != BT_CONNECTED)
return false;
^ permalink raw reply related
* Re: [RFC PATCH 2/5] mlx5: Add support for UDP tunnel segmentation with outer checksum offload
From: Saeed Mahameed @ 2016-04-29 19:27 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Finlay, Alexander Duyck, Eugenia Emantayev, Bruce W Allan,
Saeed Mahameed, Linux Netdev List, intel-wired-lan, Ariel Elior,
Michael Chan
In-Reply-To: <CAKgT0UeGJY0kntN6KeXmqZEY3k=uND4RXY+RG94q0Da2Tt2rWg@mail.gmail.com>
On Fri, Apr 29, 2016 at 4:59 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 6:18 PM, Matthew Finlay <Matt@mellanox.com> wrote:
>>
>>
>>
>>
>>
>>>>
>>>> The mlx5 hardware requires the outer UDP checksum is not set when offloading encapsulated packets.
>>>
>>>The Intel documentation said the same thing. That was due to the fact
>>>that the hardware didn't computer the outer UDP header checksum. I
>>>suspect the Mellanox hardware has the same issue. Also I have tested
>>>on a ConnectX-4 board with the latest firmware and what I am seeing is
>>>that with my patches applied the outer checksum is being correctly
>>>applied for segmentation offloads.
>>>
>>>My thought is that that the hardware appears to ignore the UDP
>>>checksum so if it is non-zero you cannot guarantee the checksum would
>>>be correct on the last frame if it is a different size than the rest
>>>of the segments. In the case of these patches that issue has been
>>>resolved as I have precomputed the UDP checksum for the outer UDP
>>>header and all of the segments will be the same length so there should
>>>be no variation in the UDP checksum of the outer header. Unless you
>>>can tell my exactly the reason why we cannot provide the outer UDP
>>>checksum I would assume that the reason is due to the fact that the
>>>hardware doesn't compute it so you cannot handle a fragment on the end
>>>which is resolved already via GSO_PARTIAL.
>>
>> I will check internally and verify there are no unforeseen issues with setting the outer UDP checksum in this scenario.
>
> Thanks. Any idea how long it should be. I know I was getting a
> auto-reply about people being out until May 1st due to a holiday so I
> am just wondering if we should have Dave drop this patch set and I
> submit a v2 when you can get me the feedback next week, or if we run
> with the patches as-is for now and be prepared to revert if anything
> should come up.
>
Alex,
I reviewed your patches and other than the claim about mlx4,
everything seems to be ok.
I took the liberty to apply your patches and play around with them
only on mlx5 for now,
It seems that it works and HW do correctly calculate the IPv6 outer
UDP sum as illustrated in the log from the tcpdump on the receiver
[1], I also noticed that the GSO also works on the xmitter and the HW
ignores the outer sum as expected.
Matt, I think you still need to do the homework and see if we didn't
miss anything here, but theoretically and practically what Alex did
here should work.
Tariq will also check what went wrong with mlx4 outer ipv6 support,
but this shouldn't block this series.
Side notes:
- Alex, you will need to rebase the series, it didn't apply smoothly.
- BTW mlx5 on RX side will provide csum complete and not csum unnecessary.
Saeed.
[1]
21:28:15.474287 e4:1d:2d:5c:f2:e8 > e4:1d:2d:5c:f2:d4, ethertype IPv6
(0x86dd), length 1514: (hlim 64, next-header UDP (17) payload length:
1460) 2001::37:4.58006 > 2001::36:4.4789: [udp sum ok] VXLAN, flags
[I] (0x08), vni 46
66:cb:6e:a4:31:4e > b6:bf:9b:61:31:62, ethertype IPv4 (0x0800), length
1444: (tos 0x0, ttl 64, id 64920, offset 0, flags [DF], proto TCP (6),
length 1430)
56.0.37.4.53614 > 56.0.36.4.commplex-link: Flags [.], cksum 0xd6cc
(correct), seq 1911713070:1911714448, ack 1, win 218, options
[nop,nop,TS val 183853011 ecr 183840106], length 1378
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
From: David Miller @ 2016-04-29 19:33 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, jhs
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 28 Apr 2016 17:52:46 +0200
> This set adds support for bridge per-vlan statistics.
Between the counter bumps in fast paths and new levels of pointer
indirection in order to RCU things, I have to agree with Stephen
that this new overhead is really pushing it.
All of this new overhead contributes to the transactional overhead
for every single packet.
Sorry I'm not going to apply this for now, unless you can come up
with something significantly cheaper.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 2/5] mlx5: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-04-29 19:36 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Matthew Finlay, Alexander Duyck, Eugenia Emantayev, Bruce W Allan,
Saeed Mahameed, Linux Netdev List, intel-wired-lan, Ariel Elior,
Michael Chan
In-Reply-To: <CALzJLG8yme_8a+aH+2VT8atq0wrEK208K1HB-iSQUtDipAyPXA@mail.gmail.com>
On Fri, Apr 29, 2016 at 12:27 PM, Saeed Mahameed
<saeedm@dev.mellanox.co.il> wrote:
> On Fri, Apr 29, 2016 at 4:59 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Thu, Apr 28, 2016 at 6:18 PM, Matthew Finlay <Matt@mellanox.com> wrote:
>>>
>>>
>>>
>>>
>>>
>>>>>
>>>>> The mlx5 hardware requires the outer UDP checksum is not set when offloading encapsulated packets.
>>>>
>>>>The Intel documentation said the same thing. That was due to the fact
>>>>that the hardware didn't computer the outer UDP header checksum. I
>>>>suspect the Mellanox hardware has the same issue. Also I have tested
>>>>on a ConnectX-4 board with the latest firmware and what I am seeing is
>>>>that with my patches applied the outer checksum is being correctly
>>>>applied for segmentation offloads.
>>>>
>>>>My thought is that that the hardware appears to ignore the UDP
>>>>checksum so if it is non-zero you cannot guarantee the checksum would
>>>>be correct on the last frame if it is a different size than the rest
>>>>of the segments. In the case of these patches that issue has been
>>>>resolved as I have precomputed the UDP checksum for the outer UDP
>>>>header and all of the segments will be the same length so there should
>>>>be no variation in the UDP checksum of the outer header. Unless you
>>>>can tell my exactly the reason why we cannot provide the outer UDP
>>>>checksum I would assume that the reason is due to the fact that the
>>>>hardware doesn't compute it so you cannot handle a fragment on the end
>>>>which is resolved already via GSO_PARTIAL.
>>>
>>> I will check internally and verify there are no unforeseen issues with setting the outer UDP checksum in this scenario.
>>
>> Thanks. Any idea how long it should be. I know I was getting a
>> auto-reply about people being out until May 1st due to a holiday so I
>> am just wondering if we should have Dave drop this patch set and I
>> submit a v2 when you can get me the feedback next week, or if we run
>> with the patches as-is for now and be prepared to revert if anything
>> should come up.
>>
>
> Alex,
>
> I reviewed your patches and other than the claim about mlx4,
> everything seems to be ok.
> I took the liberty to apply your patches and play around with them
> only on mlx5 for now,
> It seems that it works and HW do correctly calculate the IPv6 outer
> UDP sum as illustrated in the log from the tcpdump on the receiver
> [1], I also noticed that the GSO also works on the xmitter and the HW
> ignores the outer sum as expected.
>
> Matt, I think you still need to do the homework and see if we didn't
> miss anything here, but theoretically and practically what Alex did
> here should work.
> Tariq will also check what went wrong with mlx4 outer ipv6 support,
> but this shouldn't block this series.
>
> Side notes:
> - Alex, you will need to rebase the series, it didn't apply smoothly.
> - BTW mlx5 on RX side will provide csum complete and not csum unnecessary.
>
> Saeed.
>
> [1]
> 21:28:15.474287 e4:1d:2d:5c:f2:e8 > e4:1d:2d:5c:f2:d4, ethertype IPv6
> (0x86dd), length 1514: (hlim 64, next-header UDP (17) payload length:
> 1460) 2001::37:4.58006 > 2001::36:4.4789: [udp sum ok] VXLAN, flags
> [I] (0x08), vni 46
> 66:cb:6e:a4:31:4e > b6:bf:9b:61:31:62, ethertype IPv4 (0x0800), length
> 1444: (tos 0x0, ttl 64, id 64920, offset 0, flags [DF], proto TCP (6),
> length 1430)
> 56.0.37.4.53614 > 56.0.36.4.commplex-link: Flags [.], cksum 0xd6cc
> (correct), seq 1911713070:1911714448, ack 1, win 218, options
> [nop,nop,TS val 183853011 ecr 183840106], length 1378
Thanks. I'll probably resubmit next week after your latest set of 12
patches gets applied.
- Alex
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
From: Nikolay Aleksandrov @ 2016-04-29 19:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, roopa, stephen, jhs
In-Reply-To: <20160429.153326.652947047579122837.davem@davemloft.net>
On 04/29/2016 09:33 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Thu, 28 Apr 2016 17:52:46 +0200
>
>> This set adds support for bridge per-vlan statistics.
>
> Between the counter bumps in fast paths and new levels of pointer
> indirection in order to RCU things, I have to agree with Stephen
> that this new overhead is really pushing it.
>
> All of this new overhead contributes to the transactional overhead
> for every single packet.
>
> Sorry I'm not going to apply this for now, unless you can come up
> with something significantly cheaper.
>
> Thanks.
>
Okay, thanks for the feedback. Is this about the RCUfication of the pvid ?
Because that is not needed for the per-vlan stats to work, I did to unify the paths
and simplify the pvid code but I can easily drop it and revert back to using
the direct pvid id.
The only fetch will be the stats per-cpu pointer then. Would that be acceptable ?
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v4 net-next 2/2] ppp: add rtnetlink device creation support
From: David Miller @ 2016-04-29 20:10 UTC (permalink / raw)
To: g.nault; +Cc: netdev, linux-ppp, paulus, stephen, wharms
In-Reply-To: <20160428155530.GA24359@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Thu, 28 Apr 2016 17:55:30 +0200
> Define PPP device handler for use with rtnetlink.
> The only PPP specific attribute is IFLA_PPP_DEV_FD. It is mandatory and
> contains the file descriptor of the associated /dev/ppp instance (the
> file descriptor which would have been used for ioctl(PPPIOCNEWUNIT) in
> the ioctl-based API). The PPP device is removed when this file
> descriptor is released (same behaviour as with ioctl based PPP
> devices).
>
> PPP devices created with the rtnetlink API behave like the ones created
> with ioctl(PPPIOCNEWUNIT). In particular existing ioctls work the same
> way, no matter how the PPP device was created.
> The rtnl callbacks are also assigned to ioctl based PPP devices. This
> way, rtnl messages have the same effect on any PPP devices.
> The immediate effect is that all PPP devices, even ioctl-based
> ones, can now be removed with "ip link del".
>
> A minor difference still exists between ioctl and rtnl based PPP
> interfaces: in the device name, the number following the "ppp" prefix
> corresponds to the PPP unit number for ioctl based devices, while it is
> just an unrelated incrementing index for rtnl ones.
>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Series applied, thanks for doing this work!
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
From: David Miller @ 2016-04-29 20:12 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, jhs
In-Reply-To: <5723BABD.4080005@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri, 29 Apr 2016 21:49:17 +0200
> Because that is not needed for the per-vlan stats to work, I did to
> unify the paths and simplify the pvid code but I can easily drop it
> and revert back to using the direct pvid id. The only fetch will be
> the stats per-cpu pointer then. Would that be acceptable ?
It would be a step in the right direction, for sure.
^ permalink raw reply
* Re: [PATCH net-next] net: constify is_skb_forwardable's arguments
From: David Miller @ 2016-04-29 20:15 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa
In-Reply-To: <1461859168-6217-1-git-send-email-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 28 Apr 2016 17:59:28 +0200
> is_skb_forwardable is not supposed to change anything so constify its
> arguments
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
From: Nikolay Aleksandrov @ 2016-04-29 20:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev, roopa, stephen, jhs
In-Reply-To: <20160429.161208.1214717753456694357.davem@davemloft.net>
On 04/29/2016 10:12 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Fri, 29 Apr 2016 21:49:17 +0200
>
>> Because that is not needed for the per-vlan stats to work, I did to
>> unify the paths and simplify the pvid code but I can easily drop it
>> and revert back to using the direct pvid id. The only fetch will be
>> the stats per-cpu pointer then. Would that be acceptable ?
>
> It would be a step in the right direction, for sure.
>
Okay, just one more thing I forgot to mention - please note that my code swaps
an unconditional smp_rmb() (in br_get_pvid()) for a pointer fetch, I'm not sure
the pointer fetch is slower as it's probably already in the cache if that vlan
is used.
Anyway, I will resubmit without that patch.
Thanks,
Nik
^ permalink raw reply
* Re: [PATCH v3 net-next 0/2] net: ethernet: enc28j60: small improvements
From: David Miller @ 2016-04-29 20:23 UTC (permalink / raw)
To: mhei; +Cc: jic23, afd, broonie, netdev
In-Reply-To: <1461873975-6368-1-git-send-email-mhei@heimpold.de>
From: Michael Heimpold <mhei@heimpold.de>
Date: Thu, 28 Apr 2016 22:06:13 +0200
> This series of two patches adds the following improvements to the driver:
>
> 1) Rework the central SPI read function so that it is compatible with
> SPI masters which only support half duplex transfers.
>
> 2) Add a device tree binding for the driver.
>
> Changelog:
>
> v3: * renamed and improved binding documentation as
> suggested by Rob Herring
>
> v2: * took care of Arnd Bergmann's review comments
> - allow to specify MAC address via DT
> - unconditionally define DT id table
> * increased the driver version minor number
> * driver author's email address bounces, removed from address list
>
> v1: * Initial submission
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next V1 00/11] Mellanox 100G extending mlx5 ethtool support
From: Saeed Mahameed @ 2016-04-29 20:27 UTC (permalink / raw)
To: David Miller
Cc: Saeed Mahameed, Linux Netdev List, Or Gerlitz, Tal Alon,
Eran Ben Elisha
In-Reply-To: <20160426.174150.2239494413467110.davem@davemloft.net>
On Wed, Apr 27, 2016 at 12:41 AM, David Miller <davem@davemloft.net> wrote:
> From: Saeed Mahameed <saeedm@dev.mellanox.co.il>
> Date: Tue, 26 Apr 2016 23:55:03 +0300
>
>> It will be a nightmare to rollback in such case. What if the rollback failed ?
>
> It is absolutely essential to handle this properly.
>
> Which means you must have a prepare/commit model, wherein the prepare
> phase makes sure to pre-allocate all necessary resources, and only if
> all the prepare phase preparations succeed will the commit phase run.
>
> The commit phase cannot error, because all of the resources have been
> allocated successfully already.
>
> This way there are no issues of "rolling back" because you never
> actually move the state forward until you can guarantee that you can
> do everything.
Right, for pure software/kernel resources this is the right way to go,
Actually we already have a patch that is similar of what you described,
we are aiming to push it towards 4.8.
but my concerns is when features A and B requires firmware commands A then B
and firmware command B fails, there is no gurantee that roll back for
firmware command A will work.
this is why in case of B fails we keep the state (new A and prev B)
rather than try to go back to (prev A and prev B).
^ permalink raw reply
* Re: [PATCH net-next 00/12] Mellanox 100G mlx5 ethernet aRFS support
From: David Miller @ 2016-04-29 20:31 UTC (permalink / raw)
To: saeedm; +Cc: netdev, ogerlitz, talal, eranbe
In-Reply-To: <1461883002-8912-1-git-send-email-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 29 Apr 2016 01:36:30 +0300
> This series adds accelerated RFS support for the mlx5e driver.
Series applied, thanks Saeed.
^ permalink raw reply
* Re: [PATCH net-next V1 00/11] Mellanox 100G extending mlx5 ethtool support
From: David Miller @ 2016-04-29 20:34 UTC (permalink / raw)
To: saeedm; +Cc: saeedm, netdev, ogerlitz, talal, eranbe
In-Reply-To: <CALzJLG_a4s54MwCzOHMeCp0i5F5pyb-PHY_AFA-ok2JT5S62wQ@mail.gmail.com>
From: Saeed Mahameed <saeedm@dev.mellanox.co.il>
Date: Fri, 29 Apr 2016 23:27:06 +0300
> but my concerns is when features A and B requires firmware commands A then B
> and firmware command B fails, there is no gurantee that roll back for
> firmware command A will work.
>
> this is why in case of B fails we keep the state (new A and prev B)
> rather than try to go back to (prev A and prev B).
That's a limitation of your firmware I would say.
Users do not expect the semantics you will be providing, if "change A and B"
fails both states must not be changed.
This is an unwavering requirement, you must do everything you can to
meet that expection.
You cannot say "our firmware does this so, you might get partial
updates." That simply is not acceptable.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/7] net: make TCP preemptible
From: David Miller @ 2016-04-29 20:39 UTC (permalink / raw)
To: edumazet; +Cc: netdev, soheil, ast, marcelo.leitner, eric.dumazet
In-Reply-To: <1461899449-8096-1-git-send-email-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 28 Apr 2016 20:10:42 -0700
> Most of TCP stack assumed it was running from BH handler.
Assuming you are respinning this to fix that stats bumping typo.
You should really look into how that got corrupted. :)
^ 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