Netdev List
 help / color / mirror / Atom feed
* Re: Issue with RTL8111 NIC after upgrade to kernel 4.19
From: Heiner Kallweit @ 2018-11-22 19:53 UTC (permalink / raw)
  To: Marc Dionne
  Cc: andrew, norbert.jurkeit, nic_swsd, Florian Fainelli, David Miller,
	netdev, Linux Kernel Mailing List, michael.wiktowy, jcline
In-Reply-To: <CAB9dFdvscM_vjhautiNc6yNZ4_aMQOkcGeKmAyRv4E191q9jaQ@mail.gmail.com>

On 22.11.2018 20:29, Marc Dionne wrote:
> On Thu, Nov 22, 2018 at 2:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Thanks a lot for testing. Could you please test also the following
>> as an alternative to the delay?
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 55202a0ac..aeccb2323 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -2254,6 +2254,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
>>         new_driver->mdiodrv.driver.probe = phy_probe;
>>         new_driver->mdiodrv.driver.remove = phy_remove;
>>         new_driver->mdiodrv.driver.owner = owner;
>> +       new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
>>
>>         retval = driver_register(&new_driver->mdiodrv.driver);
>>         if (retval) {
> 
> That also gets me network reliably, switching between a kernel where
> it fails (distro 4.19 kernel) and the custom kernel with the patch.
> 
Thanks again for the very quick response. The result is good and bad
news at once: good because it seems we have a fix, bad because we
don't understand the root cause of the issue yet.

>>> There's a side issue that network startup is taking a full minute
>>> longer than it should, but that's possibly unrelated.
> 
> BTW that's an unrelated rng issue, for which I have a workaround.
> 
Indeed totally unrelated, but sounds familiar. I had the same issue
on a headless system which therefore generates very little entropy.
I fixed it by setting CONFIG_RANDOM_TRUST_CPU=y

>>>
>> Thanks, Heiner
> 
> Marc
> 
Heiner

^ permalink raw reply

* Re: Issue with RTL8111 NIC after upgrade to kernel 4.19
From: Heiner Kallweit @ 2018-11-22 19:48 UTC (permalink / raw)
  To: Andrew Lunn, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: Marc Dionne, norbert.jurkeit, nic_swsd, Florian Fainelli,
	David Miller, netdev, Linux Kernel Mailing List, michael.wiktowy,
	jcline
In-Reply-To: <20181122185705.GG10697@lunn.ch>

On 22.11.2018 19:57, Andrew Lunn wrote:
>> Thanks a lot for testing. Could you please test also the following
>> as an alternative to the delay?
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 55202a0ac..aeccb2323 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -2254,6 +2254,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
>>         new_driver->mdiodrv.driver.probe = phy_probe;
>>         new_driver->mdiodrv.driver.remove = phy_remove;
>>         new_driver->mdiodrv.driver.owner = owner;
>> +       new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
>>
>>         retval = driver_register(&new_driver->mdiodrv.driver);
>>         if (retval) {
> 
> 
> Humm, maybe i don't understand the issue correctly.
> 
> When the MDIO bus is registered, we scan the bus looking for PHYs.
> When we find a PHY, we call phy_device_create(). That will then
> trigger the loading of the kernel module which should driver this phy.
> 
> Sometime later, the PHY driver module gets loaded and calls
> phy_drivers_register() to register the list of IDs it supports.  The
> driver core will then call phy_bus_match() to see if the newly loaded
> driver matches to a device we have created. If so, the driver will be
> associated to the device.
> 
request_module() is synchronous and should return only once the init
function of the loaded module has been executed. Means in case of
phylib: all PHY drivers of the module have been registered

> Sometime later, the MAC tries to attach to the phy using
> phy_attach_direct(). If there is no driver associated to the device,
> we use the generic PHY driver.
> 
> I thought the issue was the race condition between loading the module
> and the MAC attaching to it? We are getting the generic driver because
> the specific driver is still loading?
> 
No. The issue happens before: For whatever reason the PHY driver doesn't
bind to the PHY device, even though they match. Therefore
phy_attach_direct() then binds the genphy driver.

In theory it shouldn't make a difference whether device or driver is
registered first. When a driver is registered it checks all unbound
devices on the same bus for a match.

Standard is asynchronous driver probing, therefore in case of phylib
(in at least a lot of cases) the driver probes the devices on the bus
once the PHY device was registered. This seems to fail and I have no
idea why.

It has been reported that loading the PHY driver module upfront fixes
the issue. Therefore I assumed it may help to let the PHY driver probe
for devices earlier (even though the device isn't even registered yet).
And indeed (see mail just sent by Marc) using synchronous probing
fixes the issue.
It seems to me that probing triggered from device registering and
probing triggered from driver registering somehow interfere.

Of course it's not very satisfying to have a fix but to not understand
the root cause of the issue. I add Greg and Rafael as maintainers of
the driver core to the discussion. Maybe they can shed some light on
the situation.

> If that really is the issue, i think phy_attach_direct() should try
> loading the module again, doing it synchronously. Only when it fails
> should the generic driver be associated to the device.
> 
>        Andrew
> 
> 
Heiner

^ permalink raw reply

* BUG: corrupted list in p9_read_work (2)
From: syzbot @ 2018-11-23  6:29 UTC (permalink / raw)
  To: asmadeus, davem, ericvh, linux-kernel, lucho, netdev,
	syzkaller-bugs, v9fs-developer

Hello,

syzbot found the following crash on:

HEAD commit:    edeca3a769ad Merge tag 'sound-4.20-rc4' of git://git.kerne..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13dee26d400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=73e2bc0cb6463446
dashboard link: https://syzkaller.appspot.com/bug?extid=77a25acfa0382e06ab23
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1459062b400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+77a25acfa0382e06ab23@syzkaller.appspotmail.com

8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
list_del corruption, ffff8881d12ddfb0->next is LIST_POISON1  
(dead000000000100)
------------[ cut here ]------------
kernel BUG at lib/list_debug.c:47!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 2915 Comm: kworker/0:2 Not tainted 4.20.0-rc3+ #124
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: events p9_read_work
RIP: 0010:__list_del_entry_valid.cold.1+0x26/0x4a lib/list_debug.c:45
Code: d2 fd 0f 0b 4c 89 e2 48 89 de 48 c7 c7 00 d1 60 88 e8 c6 37 d2 fd 0f  
0b 4c 89 ea 48 89 de 48 c7 c7 60 d0 60 88 e8 b2 37 d2 fd <0f> 0b 48 89 de  
48 c7 c7 c0 d1 60 88 e8 a1 37 d2 fd 0f 0b 48 89 de
RSP: 0018:ffff8881cb9075c0 EFLAGS: 00010286
RAX: 000000000000004e RBX: ffff8881d12ddfb0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8165eaf5 RDI: 0000000000000005
RBP: ffff8881cb9075d8 R08: ffff8881cbb30400 R09: ffffed103b5c5020
R10: ffffed103b5c5020 R11: ffff8881dae28107 R12: dead000000000200
R13: dead000000000100 R14: ffff8881d8fd6a90 R15: ffff8881d8fd6a50
FS:  0000000000000000(0000) GS:ffff8881dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000001b3f06000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  __list_del_entry include/linux/list.h:117 [inline]
  list_del include/linux/list.h:125 [inline]
  p9_read_work+0xa95/0x1140 net/9p/trans_fd.c:378
  process_one_work+0xc90/0x1c40 kernel/workqueue.c:2153
  worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
  kthread+0x35a/0x440 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace e66a284578a06221 ]---
RIP: 0010:__list_del_entry_valid.cold.1+0x26/0x4a lib/list_debug.c:45
Code: d2 fd 0f 0b 4c 89 e2 48 89 de 48 c7 c7 00 d1 60 88 e8 c6 37 d2 fd 0f  
0b 4c 89 ea 48 89 de 48 c7 c7 60 d0 60 88 e8 b2 37 d2 fd <0f> 0b 48 89 de  
48 c7 c7 c0 d1 60 88 e8 a1 37 d2 fd 0f 0b 48 89 de
RSP: 0018:ffff8881cb9075c0 EFLAGS: 00010286
RAX: 000000000000004e RBX: ffff8881d12ddfb0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8165eaf5 RDI: 0000000000000005
RBP: ffff8881cb9075d8 R08: ffff8881cbb30400 R09: ffffed103b5c5020
R10: ffffed103b5c5020 R11: ffff8881dae28107 R12: dead000000000200
R13: dead000000000100 R14: ffff8881d8fd6a90 R15: ffff8881d8fd6a50
FS:  0000000000000000(0000) GS:ffff8881dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000001b3f06000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH net] net: thunderx: set xdp_prog to NULL if bpf_prog_add fails
From: David Miller @ 2018-11-22 19:41 UTC (permalink / raw)
  To: lorenzo.bianconi; +Cc: netdev, Sunil.Goutham
In-Reply-To: <e18b8d70c5fff9cdf5ef8c9e9dba31b27fd03040.1542809524.git.lorenzo.bianconi@redhat.com>

From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Wed, 21 Nov 2018 16:32:10 +0100

> Set xdp_prog pointer to NULL if bpf_prog_add fails since that routine
> reports the error code instead of NULL in case of failure and xdp_prog
> pointer value is used in the driver to verify if XDP is currently
> enabled.
> Moreover report the error code to userspace if nicvf_xdp_setup fails
> 
> Fixes: 05c773f52b96 ("net: thunderx: Add basic XDP support")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net-next] {net,IB}/mlx4: Initialize CQ buffers in the driver when possible
From: David Miller @ 2018-11-22 19:39 UTC (permalink / raw)
  To: tariqt; +Cc: netdev, eranbe, danielj, jackm
In-Reply-To: <1542813125-28797-1-git-send-email-tariqt@mellanox.com>

From: Tariq Toukan <tariqt@mellanox.com>
Date: Wed, 21 Nov 2018 17:12:05 +0200

> From: Daniel Jurgens <danielj@mellanox.com>
> 
> Perform CQ initialization in the driver when the capability is supported
> by the FW.  When passing the CQ to HW indicate that the CQ buffer has
> been pre-initialized.
> 
> Doing so decreases CQ creation time.  Testing on P8 showed a single 2048
> entry CQ creation time was reduced from ~395us to ~170us, which is
> 2.3x faster.
> 
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>

Applied.

^ permalink raw reply

* [PATCH v2 bpf-next] bpf: add skb->tstamp r/w access from tc clsact and cg skb progs
From: Vlad Dumitrescu @ 2018-11-22 19:39 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev
  Cc: Eric Dumazet, Willem de Bruijn, Vlad Dumitrescu
In-Reply-To: <20181121001848.35894-1-vladum@google.com>

This could be used to rate limit egress traffic in concert with a qdisc
which supports Earliest Departure Time, such as FQ.

Write access from cg skb progs only with CAP_SYS_ADMIN, since the value
will be used by downstream qdiscs. It might make sense to relax this.

Changes v1 -> v2:
  - allow access from cg skb, write only with CAP_SYS_ADMIN

Signed-off-by: Vlad Dumitrescu <vladum@google.com>
---
 include/uapi/linux/bpf.h                    |  1 +
 net/core/filter.c                           | 29 +++++++++++++++++++++
 tools/include/uapi/linux/bpf.h              |  1 +
 tools/testing/selftests/bpf/test_verifier.c | 29 +++++++++++++++++++++
 4 files changed, 60 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c1554aa074659..23e2031a43d43 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2468,6 +2468,7 @@ struct __sk_buff {
 
 	__u32 data_meta;
 	struct bpf_flow_keys *flow_keys;
+	__u64 tstamp;
 };
 
 struct bpf_tunnel_key {
diff --git a/net/core/filter.c b/net/core/filter.c
index f6ca38a7d4332..65dc13aeca7c4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5573,6 +5573,10 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
 		if (size != sizeof(struct bpf_flow_keys *))
 			return false;
 		break;
+	case bpf_ctx_range(struct __sk_buff, tstamp):
+		if (size != sizeof(__u64))
+			return false;
+		break;
 	default:
 		/* Only narrow read access allowed for now. */
 		if (type == BPF_WRITE) {
@@ -5600,6 +5604,7 @@ static bool sk_filter_is_valid_access(int off, int size,
 	case bpf_ctx_range(struct __sk_buff, data_end):
 	case bpf_ctx_range(struct __sk_buff, flow_keys):
 	case bpf_ctx_range_till(struct __sk_buff, family, local_port):
+	case bpf_ctx_range(struct __sk_buff, tstamp):
 		return false;
 	}
 
@@ -5638,6 +5643,10 @@ static bool cg_skb_is_valid_access(int off, int size,
 		case bpf_ctx_range(struct __sk_buff, priority):
 		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
 			break;
+		case bpf_ctx_range(struct __sk_buff, tstamp):
+			if (!capable(CAP_SYS_ADMIN))
+				return false;
+			break;
 		default:
 			return false;
 		}
@@ -5665,6 +5674,7 @@ static bool lwt_is_valid_access(int off, int size,
 	case bpf_ctx_range_till(struct __sk_buff, family, local_port):
 	case bpf_ctx_range(struct __sk_buff, data_meta):
 	case bpf_ctx_range(struct __sk_buff, flow_keys):
+	case bpf_ctx_range(struct __sk_buff, tstamp):
 		return false;
 	}
 
@@ -5874,6 +5884,7 @@ static bool tc_cls_act_is_valid_access(int off, int size,
 		case bpf_ctx_range(struct __sk_buff, priority):
 		case bpf_ctx_range(struct __sk_buff, tc_classid):
 		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+		case bpf_ctx_range(struct __sk_buff, tstamp):
 			break;
 		default:
 			return false;
@@ -6093,6 +6104,7 @@ static bool sk_skb_is_valid_access(int off, int size,
 	case bpf_ctx_range(struct __sk_buff, tc_classid):
 	case bpf_ctx_range(struct __sk_buff, data_meta):
 	case bpf_ctx_range(struct __sk_buff, flow_keys):
+	case bpf_ctx_range(struct __sk_buff, tstamp):
 		return false;
 	}
 
@@ -6179,6 +6191,7 @@ static bool flow_dissector_is_valid_access(int off, int size,
 	case bpf_ctx_range(struct __sk_buff, tc_classid):
 	case bpf_ctx_range(struct __sk_buff, data_meta):
 	case bpf_ctx_range_till(struct __sk_buff, family, local_port):
+	case bpf_ctx_range(struct __sk_buff, tstamp):
 		return false;
 	}
 
@@ -6488,6 +6501,22 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
 		*insn++ = BPF_LDX_MEM(BPF_SIZEOF(void *), si->dst_reg,
 				      si->src_reg, off);
 		break;
+
+	case offsetof(struct __sk_buff, tstamp):
+		BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, tstamp) != 8);
+
+		if (type == BPF_WRITE)
+			*insn++ = BPF_STX_MEM(BPF_DW,
+					      si->dst_reg, si->src_reg,
+					      bpf_target_off(struct sk_buff,
+							     tstamp, 8,
+							     target_size));
+		else
+			*insn++ = BPF_LDX_MEM(BPF_DW,
+					      si->dst_reg, si->src_reg,
+					      bpf_target_off(struct sk_buff,
+							     tstamp, 8,
+							     target_size));
 	}
 
 	return insn - insn_buf;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index c1554aa074659..23e2031a43d43 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2468,6 +2468,7 @@ struct __sk_buff {
 
 	__u32 data_meta;
 	struct bpf_flow_keys *flow_keys;
+	__u64 tstamp;
 };
 
 struct bpf_tunnel_key {
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 54d16fbdef8b9..537a8f91af02d 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -2446,6 +2446,10 @@ static struct bpf_test tests[] = {
 				    offsetof(struct __sk_buff, tc_index)),
 			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
 				    offsetof(struct __sk_buff, cb[3])),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
+				    offsetof(struct __sk_buff, tstamp)),
+			BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+				    offsetof(struct __sk_buff, tstamp)),
 			BPF_EXIT_INSN(),
 		},
 		.errstr_unpriv = "",
@@ -5297,6 +5301,31 @@ static struct bpf_test tests[] = {
 		.errstr_unpriv = "R2 leaks addr into helper function",
 		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
 	},
+	{
+		"write tstamp from CGROUP_SKB",
+		.insns = {
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+				    offsetof(struct __sk_buff, tstamp)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.result_unpriv = REJECT,
+		.errstr_unpriv = "invalid bpf_context access off=152 size=8",
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"read tstamp from CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
+				    offsetof(struct __sk_buff, tstamp)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
 	{
 		"multiple registers share map_lookup_elem result",
 		.insns = {
-- 
2.19.1.1215.g8438c0b245-goog

^ permalink raw reply related

* Re: [PATCH net] net/dim: Update DIM start sample after each DIM iteration
From: David Miller @ 2018-11-22 19:37 UTC (permalink / raw)
  To: talgi; +Cc: netdev, tariqt, saeedm, f.fainelli, andrew.gospodarek
In-Reply-To: <1542810503-51548-1-git-send-email-talgi@mellanox.com>

From: Tal Gilboa <talgi@mellanox.com>
Date: Wed, 21 Nov 2018 16:28:23 +0200

> On every iteration of net_dim, the algorithm may choose to
> check for the system state by comparing current data sample
> with previous data sample. After each of these comparison,
> regardless of the action taken, the sample used as baseline
> is needed to be updated.
> 
> This patch fixes a bug that causes DIM to take wrong decisions,
> due to never updating the baseline sample for comparison between
> iterations. This way, DIM always compares current sample with
> zeros.
> 
> Although this is a functional fix, it also improves and stabilizes
> performance as the algorithm works properly now.
> 
> Performance:
> Tested single UDP TX stream with pktgen:
> samples/pktgen/pktgen_sample03_burst_single_flow.sh -i p4p2 -d 1.1.1.1
> -m 24:8a:07:88:26:8b -f 3 -b 128
> 
> ConnectX-5 100GbE packet rate improved from 15-19Mpps to 19-20Mpps.
> Also, toggling between profiles is less frequent with the fix.
> 
> Fixes: 8115b750dbcb ("net/dim: use struct net_dim_sample as arg to net_dim")
> Signed-off-by: Tal Gilboa <talgi@mellanox.com>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: bridge: add support for user-controlled bool options
From: Stephen Hemminger @ 2018-11-22 19:37 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: Andrew Lunn, netdev, roopa, davem, bridge
In-Reply-To: <e2d49e09-ab36-7daa-9e88-178025bedcc7@cumulusnetworks.com>

On Thu, 22 Nov 2018 18:01:29 +0200
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> On 22/11/2018 17:35, Andrew Lunn wrote:
> > On Thu, Nov 22, 2018 at 06:29:24AM +0200, Nikolay Aleksandrov wrote:  
> >> We have been adding many new bridge options, a big number of which are
> >> boolean but still take up netlink attribute ids and waste space in the skb.
> >> Recently we discussed learning from link-local packets[1] and decided
> >> yet another new boolean option will be needed, thus introducing this API
> >> to save some bridge nl space.
> >> The API supports changing the value of multiple boolean options at once
> >> via the br_boolopt_multi struct which has an optmask (which options to
> >> set, bit per opt) and optval (options' new values). Future boolean
> >> options will only be added to the br_boolopt_id enum and then will have
> >> to be handled in br_boolopt_toggle/get. The API will automatically
> >> add the ability to change and export them via netlink, sysfs can use the
> >> single boolopt function versions to do the same. The behaviour with
> >> failing/succeeding is the same as with normal netlink option changing.
> >>
> >> If an option requires mapping to internal kernel flag or needs special
> >> configuration to be enabled then it should be handled in
> >> br_boolopt_toggle. It should also be able to retrieve an option's current
> >> state via br_boolopt_get.
> >>
> >> [1] https://www.spinics.net/lists/netdev/msg532698.html
> >>
> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> >> ---
> >>  include/uapi/linux/if_bridge.h | 18 +++++++++
> >>  include/uapi/linux/if_link.h   |  1 +
> >>  net/bridge/br.c                | 68 ++++++++++++++++++++++++++++++++++
> >>  net/bridge/br_netlink.c        | 17 ++++++++-
> >>  net/bridge/br_private.h        |  6 +++
> >>  net/core/rtnetlink.c           |  2 +-
> >>  6 files changed, 110 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >> index e41eda3c71f1..6dc02c03bdf8 100644
> >> --- a/include/uapi/linux/if_bridge.h
> >> +++ b/include/uapi/linux/if_bridge.h
> >> @@ -292,4 +292,22 @@ struct br_mcast_stats {
> >>  	__u64 mcast_bytes[BR_MCAST_DIR_SIZE];
> >>  	__u64 mcast_packets[BR_MCAST_DIR_SIZE];
> >>  };
> >> +
> >> +/* bridge boolean options
> >> + * IMPORTANT: if adding a new option do not forget to handle
> >> + *            it in br_boolopt_toggle/get and bridge sysfs
> >> + */
> >> +enum br_boolopt_id {
> >> +	BR_BOOLOPT_MAX
> >> +};
> >> +
> >> +/* struct br_boolopt_multi - change multiple bridge boolean options
> >> + *
> >> + * @optval: new option values (bit per option)
> >> + * @optmask: options to change (bit per option)
> >> + */
> >> +struct br_boolopt_multi {
> >> +	__u32 optval;
> >> +	__u32 optmask;
> >> +};  
> > 
> > Hi Nikolay
> > 
> > Thanks for handling this.
> > 
> > How many boolean options do we already have? What it the likelihood a
> > u32 is going to be too small, in a couple of years time?
> >   
> 
> It would mean doubling the number of current options and this is only for
> boolean options so I think we're safe.
> 
> > I recently went through the pain of converting the u32 for
> > representing link modes in the phylib API to a linux bitmap.  I'm just
> > wondering if in the long run, using a linux bitmap right from the
> > beginning would be better?
> >   
> >> +int br_boolopt_multi_toggle(struct net_bridge *br,
> >> +			    struct br_boolopt_multi *bm)
> >> +{
> >> +	unsigned long bitmap = bm->optmask;
> >> +	int err = 0;
> >> +	int opt_id;
> >> +
> >> +	for_each_set_bit(opt_id, &bitmap, BR_BOOLOPT_MAX) {
> >> +		bool on = !!(bm->optval & BIT(opt_id));
> >> +
> >> +		err = br_boolopt_toggle(br, opt_id, on);
> >> +		if (err) {
> >> +			br_debug(br, "boolopt multi-toggle error: option: %d current: %d new: %d error: %d\n",
> >> +				 opt_id, br_boolopt_get(br, opt_id), on, err);  
> > 
> > Would it be possible to return that to userspace using the extended
> > error infrastructure?
> >   
> 
> No, it doesn't support dynamic messages AFAIK.
> 
> >       Andrew
> >   
> 

My concern is about backwards compatibility. What about old userspace and new userspace tools with old kernels.
Having multiple bits does allow handling cases where certain combos won't work.

^ permalink raw reply

* Re: [PATCH net-next] selftests: explicitly require kernel features needed by udpgro tests
From: David Miller @ 2018-11-22 19:35 UTC (permalink / raw)
  To: pabeni; +Cc: netdev, naresh.kamboju
In-Reply-To: <f881c042dcd99eea8e6c83c00f8392aa774dbcea.1542806750.git.pabeni@redhat.com>

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 21 Nov 2018 14:31:15 +0100

> commit 3327a9c46352f1 ("selftests: add functionals test for UDP GRO")
> make use of ipv6 NAT, but such a feature is not currently implied by
> selftests. Since the 'ip[6]tables' commands may actually create nft rules,
> depending on the specific user-space version, let's pull both NF and
> NFT nat modules plus the needed deps.
> 
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Fixes: 3327a9c46352f1 ("selftests: add functionals test for UDP GRO")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH v3 net-next 13/21] net: usb: aqc111: Add support for TSO
From: David Miller @ 2018-11-22 19:17 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, andrew, Dmitry.Bezrukov
In-Reply-To: <913d4885a93b10a9ff4d8f1de318cefa54c803e6.1542794577.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Wed, 21 Nov 2018 10:13:48 +0000

> @@ -832,6 +832,10 @@ static struct sk_buff *aqc111_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
>  	/*Length of actual data*/
>  	tx_desc |= skb->len & AQ_TX_DESC_LEN_MASK;
>  
> +	/* TSO MSS */
> +	tx_desc |= ((u64)(skb_shinfo(skb)->gso_size & AQ_TX_DESC_MSS_MASK)) <<
> +		   AQ_TX_DESC_MSS_SHIFT;
 ...
> +#define AQ_TX_DESC_MSS_MASK	0x7FFF

This implies a GSO size limit, which you need to advertise properly in
netdevice->gso_max_size.

Also, where is the TSO feature flag being set to actually enable the
stack sending your GSO frames?

^ permalink raw reply

* Re: [PATCH v3 net-next 10/21] net: usb: aqc111: Add checksum offload support
From: David Miller @ 2018-11-22 19:14 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, andrew, Dmitry.Bezrukov
In-Reply-To: <61c82d162dda8c9a11e9d55aba4a311cf1ef59c6.1542794577.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Wed, 21 Nov 2018 10:13:42 +0000

> +static void aqc111_rx_checksum(struct sk_buff *skb, u64 *pkt_desc)
> +{
 ...
> +	/* checksum error bit is set */
> +	if (*pkt_desc & AQ_RX_PD_L4_ERR || *pkt_desc & AQ_RX_PD_L3_ERR)
> +		return;
 ...
> @@ -661,6 +698,7 @@ static int aqc111_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  		skb_set_tail_pointer(new_skb, new_skb->len);
>  
>  		new_skb->truesize = new_skb->len + sizeof(struct sk_buff);
> +		aqc111_rx_checksum(new_skb, pkt_desc);

This is another reason to use a 'cpu_desc' local variable to hold the
endian translated descriptor value so you don't have to dereference
this thing over and over again.

^ permalink raw reply

* Re: [PATCH v3 net-next 09/21] net: usb: aqc111: Implement RX data path
From: David Miller @ 2018-11-22 19:12 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, andrew, Dmitry.Bezrukov
In-Reply-To: <0c2bb26252ececacc6f2c23304d7aa82dfa8dc05.1542794577.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Wed, 21 Nov 2018 10:13:39 +0000

> +	desc_hdr = *(u64 *)skb_tail_pointer(skb);
> +	le64_to_cpus(&desc_hdr);

This is:

	desc_hdr = le64_to_cpup(skb_tail_pointer(skb));

> +	/* Get the first RX packet descriptor */
> +	pkt_desc = (u64 *)(skb->data + desc_offset);
> +
> +	while (pkt_count--) {
> +		u32 pkt_len = 0;
> +		u32 pkt_len_with_padd = 0;
> +
> +		le64_to_cpus(pkt_desc);

Probably better to load the translated value into a local variable
once:

		u64 cpu_desc = le64_to_cpup(pkt_desc);

and then use 'cpu_desc' instead of dereferencing "*pkt_desc" over and
over again.

> +		/* Clone SKB */
> +		new_skb = skb_clone(skb, GFP_ATOMIC);
> +
> +		if (!new_skb)
> +			goto err;
> +
> +		new_skb->len = pkt_len;
> +		skb_pull(new_skb, AQ_RX_HW_PAD);
> +		skb_set_tail_pointer(new_skb, new_skb->len);
> +
> +		new_skb->truesize = new_skb->len + sizeof(struct sk_buff);

I see lots of USB drivers doing this, but it's not correct.

We have a SKB_TRUESIZE() macro, please use it.

^ permalink raw reply

* Re: [PATCH v3 net-next 08/21] net: usb: aqc111: Implement TX data path
From: David Miller @ 2018-11-22 19:07 UTC (permalink / raw)
  To: Igor.Russkikh; +Cc: linux-usb, netdev, andrew, Dmitry.Bezrukov
In-Reply-To: <8b6748790baa1b8034c9c46ecd438c1f392854c7.1542794577.git.igor.russkikh@aquantia.com>

From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Wed, 21 Nov 2018 10:13:37 +0000

> +	if (padding_size != 0)
> +		skb_put(skb, padding_size);

I think you want to use skb_put_zero() here rather than leaving it
uninitialized.  I know it's padding, but if for some reason this leaks
onto the wire or elsewhere we'll regret not initializing this buffer.

> +	/* Copy TX header */
> +	skb_push(skb, sizeof(tx_desc));
> +	cpu_to_le64s(&tx_desc);
> +	skb_copy_to_linear_data(skb, &tx_desc, sizeof(tx_desc));

I seriously wonder if, with all of the invariants wrt. length modulus
and padding, that you can safely just go:

	__le64 *p;

	p = skb_push(skb, sizeof(tx_desc));
	*p = cpu_to_le64(tx_desc);

or something like that.
	

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: use new fw interface to get the VIN and smt index
From: Jason Gunthorpe @ 2018-11-22 18:58 UTC (permalink / raw)
  To: David Miller
  Cc: ganeshgr, netdev, nirranjan, indranil, dt, linux-rdma, swise,
	linux-crypto, atul.gupta, linux-scsi, varun, santosh
In-Reply-To: <20181122.105349.1035544140286401266.davem@davemloft.net>

On Thu, Nov 22, 2018 at 10:53:49AM -0800, David Miller wrote:
> From: Jason Gunthorpe <jgg@ziepe.ca>
> Date: Wed, 21 Nov 2018 19:46:24 -0700
> 
> > On Wed, Nov 21, 2018 at 01:40:24PM +0530, Ganesh Goudar wrote:
> >> From: Santosh Rastapur <santosh@chelsio.com>
> >> 
> >> If the fw supports returning VIN/VIVLD in FW_VI_CMD save it
> >> in port_info structure else retrieve these from viid and save
> >> them  in port_info structure. Do the same for smt_idx from
> >> FW_VI_MAC_CMD
> >> 
> >> Signed-off-by: Santosh Rastapur <santosh@chelsio.com>
> >> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> >>  drivers/crypto/chelsio/chtls/chtls_cm.c         |  3 +-
> >>  drivers/infiniband/hw/cxgb4/cm.c                |  6 +--
> >>  drivers/net/ethernet/chelsio/cxgb4/cxgb4.h      | 12 ++++-
> >>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 58 ++++++++++++-------------
> >>  drivers/net/ethernet/chelsio/cxgb4/l2t.c        | 13 +++---
> >>  drivers/net/ethernet/chelsio/cxgb4/t4_hw.c      | 46 ++++++++++++++++++--
> >>  drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h   | 20 +++++++++
> >>  drivers/scsi/cxgbi/cxgb4i/cxgb4i.c              |  3 +-
> >>  drivers/target/iscsi/cxgbit/cxgbit_cm.c         |  8 ++--
> >>  9 files changed, 114 insertions(+), 55 deletions(-)
> > 
> > Applied to for-next, but please try to write better commit messages in
> > future, explain what benifit your change is bringing.
> 
> The subject line indicates this is targetting my net-next tree, therefore
> why did you apply it to your's?

It is my mistake, it ended up in RDMA patchworks next to other RDMA
chelsio patches, and contained an IB component..

It is dropped from rdma.git now.

Thanks,
Jason

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: use new fw interface to get the VIN and smt index
From: David Miller @ 2018-11-22 18:53 UTC (permalink / raw)
  To: jgg
  Cc: ganeshgr, netdev, nirranjan, indranil, dt, linux-rdma, swise,
	linux-crypto, atul.gupta, linux-scsi, varun, santosh
In-Reply-To: <20181122024624.GA28494@ziepe.ca>

From: Jason Gunthorpe <jgg@ziepe.ca>
Date: Wed, 21 Nov 2018 19:46:24 -0700

> On Wed, Nov 21, 2018 at 01:40:24PM +0530, Ganesh Goudar wrote:
>> From: Santosh Rastapur <santosh@chelsio.com>
>> 
>> If the fw supports returning VIN/VIVLD in FW_VI_CMD save it
>> in port_info structure else retrieve these from viid and save
>> them  in port_info structure. Do the same for smt_idx from
>> FW_VI_MAC_CMD
>> 
>> Signed-off-by: Santosh Rastapur <santosh@chelsio.com>
>> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
>>  drivers/crypto/chelsio/chtls/chtls_cm.c         |  3 +-
>>  drivers/infiniband/hw/cxgb4/cm.c                |  6 +--
>>  drivers/net/ethernet/chelsio/cxgb4/cxgb4.h      | 12 ++++-
>>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 58 ++++++++++++-------------
>>  drivers/net/ethernet/chelsio/cxgb4/l2t.c        | 13 +++---
>>  drivers/net/ethernet/chelsio/cxgb4/t4_hw.c      | 46 ++++++++++++++++++--
>>  drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h   | 20 +++++++++
>>  drivers/scsi/cxgbi/cxgb4i/cxgb4i.c              |  3 +-
>>  drivers/target/iscsi/cxgbit/cxgbit_cm.c         |  8 ++--
>>  9 files changed, 114 insertions(+), 55 deletions(-)
> 
> Applied to for-next, but please try to write better commit messages in
> future, explain what benifit your change is bringing.

The subject line indicates this is targetting my net-next tree, therefore
why did you apply it to your's?

^ permalink raw reply

* [PATCH bpf] bpf: fix integer overflow in queue_stack_map
From: Alexei Starovoitov @ 2018-11-22 18:49 UTC (permalink / raw)
  To: David S . Miller
  Cc: daniel, torvalds, mauricio.vasquez, ww9210, netdev, kernel-team

fix the following issues:
- allow queue_stack_map for root only
- fix u32 max_entries overflow
- disallow value_size == 0

Reported-by: Wei Wu <ww9210@gmail.com>
Fixes: f1a2e44a3aec ("bpf: add queue and stack maps")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/queue_stack_maps.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index 8bbd72d3a121..b384ea9f3254 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -7,6 +7,7 @@
 #include <linux/bpf.h>
 #include <linux/list.h>
 #include <linux/slab.h>
+#include <linux/capability.h>
 #include "percpu_freelist.h"
 
 #define QUEUE_STACK_CREATE_FLAG_MASK \
@@ -45,8 +46,12 @@ static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
 /* Called from syscall */
 static int queue_stack_map_alloc_check(union bpf_attr *attr)
 {
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
 	/* check sanity of attributes */
 	if (attr->max_entries == 0 || attr->key_size != 0 ||
+	    attr->value_size == 0 ||
 	    attr->map_flags & ~QUEUE_STACK_CREATE_FLAG_MASK)
 		return -EINVAL;
 
@@ -63,15 +68,10 @@ static struct bpf_map *queue_stack_map_alloc(union bpf_attr *attr)
 {
 	int ret, numa_node = bpf_map_attr_numa_node(attr);
 	struct bpf_queue_stack *qs;
-	u32 size, value_size;
-	u64 queue_size, cost;
-
-	size = attr->max_entries + 1;
-	value_size = attr->value_size;
-
-	queue_size = sizeof(*qs) + (u64) value_size * size;
+	u64 size, queue_size, cost;
 
-	cost = queue_size;
+	size = (u64) attr->max_entries + 1;
+	cost = queue_size = sizeof(*qs) + size * attr->value_size;
 	if (cost >= U32_MAX - PAGE_SIZE)
 		return ERR_PTR(-E2BIG);
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
From: Andrew Lunn @ 2018-11-22 18:33 UTC (permalink / raw)
  To: Anssi Hannula; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <3b502aeb-3362-22e4-d160-2ddd8371a389@bitwise.fi>

On Thu, Nov 22, 2018 at 12:40:25PM +0200, Anssi Hannula wrote:
> Hi,
> 
> On 12.9.2018 2:53, Andrew Lunn wrote:
> > Many Ethernet MAC drivers want to limit the PHY to only advertise a
> > maximum speed of 100Mbs or 1Gbps. Rather than using a mask, make use
> > of the helper function phy_set_max_speed().
> 
> But what if the PHY does not support 1Gbps in the first place?

Yes, you are correct. __set_phy_supported() needs modifying to take
into account what the PHY can do.

Thanks for pointing this out. I will take a look.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next 4/4] octeontx2-af: Bringup CGX LMAC links by default
From: Andrew Lunn @ 2018-11-22 18:26 UTC (permalink / raw)
  To: Linu Cherian; +Cc: netdev, davem, Sunil.Goutham, Linu Cherian
In-Reply-To: <20181122114837.1030-5-linuc.decode@gmail.com>

On Thu, Nov 22, 2018 at 05:18:37PM +0530, Linu Cherian wrote:
> From: Linu Cherian <lcherian@marvell.com>
> 
> - Added new CGX firmware interface API for sending link up/down
>   commands
> 
> - Do link up for cgx lmac ports by default at the time of CGX
>   driver probe.

Hi Linu

This is a complex driver which i don't understand...

By link up, do you mean the equivalent of 'ip link set up dev ethX'?

   Andrew

^ permalink raw reply

* Re: [PATCH net-next 2/3] tcp: implement coalescing on backlog queue
From: Eric Dumazet @ 2018-11-22 18:21 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: David Miller, netdev, Jean-Louis Dupond, Yuchung Cheng,
	Eric Dumazet
In-Reply-To: <CANn89iJn7+C3He8RO3X9PS6k0rGH+mHgTMyc27xC3k+ge2JhnQ@mail.gmail.com>

On Thu, Nov 22, 2018 at 10:16 AM Eric Dumazet <edumazet@google.com> wrote:

> Yes, I was considering properly filtering SACK as a refinement later [1]
> but you raise a valid point for alien stacks that are not yet using SACK :/
>
> [1] This version of the patch will not aggregate sacks since the
> memcmp() on tcp options would fail.
>
> Neal can you double check if cake_ack_filter() does not have the issue
> you just mentioned ?

Note that aggregated pure acks will have a gso_segs set to the number
of aggregated acks,
we might simply use this value later in the stack, instead of forcing
having X pure acks in the backlog
and increase memory needs and cpu costs.

Then I guess I need this fix :

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 36c9d715bf2aa7eb7bf58b045bfeb85a2ec1a696..736f7f24cdb4fe61769faaa1644c8bff01c746c4
100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1669,7 +1669,8 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
                __skb_pull(skb, hdrlen);
                if (skb_try_coalesce(tail, skb, &fragstolen, &delta)) {
                        TCP_SKB_CB(tail)->end_seq = TCP_SKB_CB(skb)->end_seq;
-                       TCP_SKB_CB(tail)->ack_seq = TCP_SKB_CB(skb)->ack_seq;
+                       if (after(TCP_SKB_CB(skb)->ack_seq,
TCP_SKB_CB(tail)->ack_seq))
+                               TCP_SKB_CB(tail)->ack_seq =
TCP_SKB_CB(skb)->ack_seq;
                        TCP_SKB_CB(tail)->tcp_flags |=
TCP_SKB_CB(skb)->tcp_flags;

                        if (TCP_SKB_CB(skb)->has_rxtstamp) {

^ permalink raw reply

* Re: [PATCH net-next 2/3] tcp: implement coalescing on backlog queue
From: Eric Dumazet @ 2018-11-22 18:16 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: David Miller, netdev, Jean-Louis Dupond, Yuchung Cheng,
	Eric Dumazet
In-Reply-To: <CADVnQykiiFz5H3J_vrCXHQ_FQ+Fg3qCZKoynsc_fdY_PcTx8cg@mail.gmail.com>

On Thu, Nov 22, 2018 at 10:01 AM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Wed, Nov 21, 2018 at 12:52 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > In case GRO is not as efficient as it should be or disabled,
> > we might have a user thread trapped in __release_sock() while
> > softirq handler flood packets up to the point we have to drop.
> >
> > This patch balances work done from user thread and softirq,
> > to give more chances to __release_sock() to complete its work.
> >
> > This also helps if we receive many ACK packets, since GRO
> > does not aggregate them.
>
> Would this coalesce duplicate incoming ACK packets? Is there a risk
> that this would eliminate incoming dupacks needed for fast recovery in
> non-SACK connections? Perhaps pure ACKs should only be coalesced if
> the ACK field is different?

Yes, I was considering properly filtering SACK as a refinement later [1]
but you raise a valid point for alien stacks that are not yet using SACK :/

[1] This version of the patch will not aggregate sacks since the
memcmp() on tcp options would fail.

Neal can you double check if cake_ack_filter() does not have the issue
you just mentioned ?

^ permalink raw reply

* Re: [PATCH v1 net] lan743x: Enable driver to work with LAN7431
From: Andrew Lunn @ 2018-11-22 18:12 UTC (permalink / raw)
  To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver
In-Reply-To: <1542828165-20331-1-git-send-email-Bryan.Whitehead@microchip.com>

On Wed, Nov 21, 2018 at 02:22:45PM -0500, Bryan Whitehead wrote:
> This driver was designed to work with both LAN7430 and LAN7431.
> The only difference between the two is the LAN7431 has support
> for external phy.
> 
> This change adds LAN7431 to the list of recognized devices
> supported by this driver.
> 
> fixes: driver won't load for LAN7431

Hi Bryan

There is a well defined format for Fixes:.

Fixes: 23f0703c125b ("lan743x: Add main source files for new lan743x driver")

       Andrew

^ permalink raw reply

* Re: [RFC v4 4/5] netdev: add netdev_is_upper_master
From: Alexis Bauvin @ 2018-11-22 18:10 UTC (permalink / raw)
  To: David Ahern, roopa; +Cc: netdev, akherbouche, Alexis Bauvin
In-Reply-To: <aba979b7-d696-57ff-edb1-efd9a4169471@cumulusnetworks.com>

Le 22 nov. 2018 à 18:14, David Ahern <dsa@cumulusnetworks.com> a écrit :
> On 11/21/18 6:07 PM, Alexis Bauvin wrote:
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 93243479085f..12459036d0da 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -7225,6 +7225,23 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
>> }
>> EXPORT_SYMBOL(netdev_lower_state_changed);
>> 
>> +/**
>> + * netdev_is_upper_master - Test if a device is a master, direct or indirect,
>> + *                          of another one.
>> + * @dev: device to start looking from
>> + * @master: device to test if master of dev
>> + */
>> +bool netdev_is_upper_master(struct net_device *dev, struct net_device *master)
>> +{
>> +	if (!dev)
>> +		return false;
>> +
>> +	if (dev->ifindex == master->ifindex)
> 
> dev == master should work as well without the dereference.

Ack, will add to next version.

>> +		return true;
>> +	return netdev_is_upper_master(netdev_master_upper_dev_get(dev), master);
>> +}
>> +EXPORT_SYMBOL(netdev_is_upper_master);
>> +
>> static void dev_change_rx_flags(struct net_device *dev, int flags)
>> {
>> 	const struct net_device_ops *ops = dev->netdev_ops;
>> 

^ permalink raw reply

* Re: [PATCH net-next 2/3] tcp: implement coalescing on backlog queue
From: Neal Cardwell @ 2018-11-22 18:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netdev, jean-louis, Yuchung Cheng, Eric Dumazet
In-Reply-To: <20181121175240.6075-3-edumazet@google.com>

On Wed, Nov 21, 2018 at 12:52 PM Eric Dumazet <edumazet@google.com> wrote:
>
> In case GRO is not as efficient as it should be or disabled,
> we might have a user thread trapped in __release_sock() while
> softirq handler flood packets up to the point we have to drop.
>
> This patch balances work done from user thread and softirq,
> to give more chances to __release_sock() to complete its work.
>
> This also helps if we receive many ACK packets, since GRO
> does not aggregate them.

Would this coalesce duplicate incoming ACK packets? Is there a risk
that this would eliminate incoming dupacks needed for fast recovery in
non-SACK connections? Perhaps pure ACKs should only be coalesced if
the ACK field is different?

neal

^ permalink raw reply

* Re: [RFC v4 3/5] vxlan: add support for underlay in non-default VRF
From: David Ahern @ 2018-11-22 17:19 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181122010713.3995-4-abauvin@scaleway.com>

On 11/21/18 6:07 PM, Alexis Bauvin wrote:
> Creating a VXLAN device with is underlay in the non-default VRF makes
> egress route lookup fail or incorrect since it will resolve in the
> default VRF, and ingress fail because the socket listens in the default
> VRF.
> 
> This patch binds the underlying UDP tunnel socket to the l3mdev of the
> lower device of the VXLAN device. This will listen in the proper VRF and
> output traffic from said l3mdev, matching l3mdev routing rules and
> looking up the correct routing table.
> 
> When the VXLAN device does not have a lower device, or the lower device
> is in the default VRF, the socket will not be bound to any interface,
> keeping the previous behaviour.
> 
> The underlay l3mdev is deduced from the VXLAN lower device
> (IFLA_VXLAN_LINK).
> 
> +----------+                         +---------+
> |          |                         |         |
> | vrf-blue |                         | vrf-red |
> |          |                         |         |
> +----+-----+                         +----+----+
>      |                                    |
>      |                                    |
> +----+-----+                         +----+----+
> |          |                         |         |
> | br-blue  |                         | br-red  |
> |          |                         |         |
> +----+-----+                         +---+-+---+
>      |                                   | |
>      |                             +-----+ +-----+
>      |                             |             |
> +----+-----+                +------+----+   +----+----+
> |          |  lower device  |           |   |         |
> |   eth0   | <- - - - - - - | vxlan-red |   | tap-red | (... more taps)
> |          |                |           |   |         |
> +----------+                +-----------+   +---------+
> 
> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> ---
>  drivers/net/vxlan.c                           | 32 +++++--
>  .../selftests/net/test_vxlan_under_vrf.sh     | 90 +++++++++++++++++++
>  2 files changed, 114 insertions(+), 8 deletions(-)
>  create mode 100755 tools/testing/selftests/net/test_vxlan_under_vrf.sh
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Thanks for adding the test case; I'll try it out next week (after the
holidays).

^ permalink raw reply

* Re: [RFC v4 4/5] netdev: add netdev_is_upper_master
From: David Ahern @ 2018-11-22 17:14 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181122010713.3995-5-abauvin@scaleway.com>

On 11/21/18 6:07 PM, Alexis Bauvin wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 93243479085f..12459036d0da 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7225,6 +7225,23 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
>  }
>  EXPORT_SYMBOL(netdev_lower_state_changed);
>  
> +/**
> + * netdev_is_upper_master - Test if a device is a master, direct or indirect,
> + *                          of another one.
> + * @dev: device to start looking from
> + * @master: device to test if master of dev
> + */
> +bool netdev_is_upper_master(struct net_device *dev, struct net_device *master)
> +{
> +	if (!dev)
> +		return false;
> +
> +	if (dev->ifindex == master->ifindex)

dev == master should work as well without the dereference.

> +		return true;
> +	return netdev_is_upper_master(netdev_master_upper_dev_get(dev), master);
> +}
> +EXPORT_SYMBOL(netdev_is_upper_master);
> +
>  static void dev_change_rx_flags(struct net_device *dev, int flags)
>  {
>  	const struct net_device_ops *ops = dev->netdev_ops;
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox