Netdev List
 help / color / mirror / Atom feed
* Re: KASAN: out-of-bounds Write in tls_push_record
From: Eric Biggers @ 2019-02-26  7:59 UTC (permalink / raw)
  To: syzbot
  Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
	syzkaller-bugs
In-Reply-To: <0000000000007567f0057072d2e7@google.com>

On Sat, Jul 07, 2018 at 06:29:03PM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    526674536360 Add linux-next specific files for 20180706
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17e63968400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=c8d1cfc0cb798e48
> dashboard link: https://syzkaller.appspot.com/bug?extid=43358359519ad16cf05e
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15790594400000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12b53f48400000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+43358359519ad16cf05e@syzkaller.appspotmail.com
> 
> RDX: 00000000fffffdef RSI: 00000000200005c0 RDI: 0000000000000004
> RBP: 00000000006cb018 R08: 0000000020000000 R09: 000000000000001c
> R10: 0000000000000040 R11: 0000000000000216 R12: 0000000000000005
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> ==================================================================
> BUG: KASAN: out-of-bounds in tls_fill_prepend include/net/tls.h:339 [inline]
> BUG: KASAN: out-of-bounds in tls_push_record+0x1091/0x1400
> net/tls/tls_sw.c:239
> Write of size 1 at addr ffff8801c07b8000 by task syz-executor985/4467
> 
> CPU: 0 PID: 4467 Comm: syz-executor985 Not tainted 4.18.0-rc3-next-20180706+
> #1
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
>  print_address_description+0x6c/0x20b mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
>  __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
>  tls_fill_prepend include/net/tls.h:339 [inline]
>  tls_push_record+0x1091/0x1400 net/tls/tls_sw.c:239
>  tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
>  tls_handle_open_record net/tls/tls_main.c:164 [inline]
>  tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
>  inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
>  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
>  __sock_release+0xd7/0x260 net/socket.c:600
>  sock_close+0x19/0x20 net/socket.c:1151
>  __fput+0x35d/0x930 fs/file_table.c:215
>  ____fput+0x15/0x20 fs/file_table.c:251
>  task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
>  exit_task_work include/linux/task_work.h:22 [inline]
>  do_exit+0x1b08/0x2750 kernel/exit.c:869
>  do_group_exit+0x177/0x440 kernel/exit.c:972
>  __do_sys_exit_group kernel/exit.c:983 [inline]
>  __se_sys_exit_group kernel/exit.c:981 [inline]
>  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:981
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x43f358
> Code: Bad RIP value.
> RSP: 002b:00007ffd4c2414b8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f358
> RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
> RBP: 00000000004bf448 R08: 00000000000000e7 R09: ffffffffffffffd0
> R10: 0000000000000040 R11: 0000000000000246 R12: 0000000000000001
> R13: 00000000006d1180 R14: 0000000000000000 R15: 0000000000000000
> 
> The buggy address belongs to the page:
> page:ffffea000701ee00 count:0 mapcount:-128 mapping:0000000000000000
> index:0x0
> flags: 0x2fffc0000000000()
> raw: 02fffc0000000000 ffffea0006b7be08 ffff88021fffac18 0000000000000000
> raw: 0000000000000000 0000000000000003 00000000ffffff7f 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  ffff8801c07b7f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>  ffff8801c07b7f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ffff8801c07b8000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>                       ^
>  ffff8801c07b8080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>  ffff8801c07b8100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==================================================================
> 
> 
> ---
> 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
> 

(As with the other reports of this...)

AFAICS this was fixed by this commit:

	commit d829e9c4112b52f4f00195900fd4c685f61365ab
	Author: Daniel Borkmann <daniel@iogearbox.net>
	Date:   Sat Oct 13 02:45:59 2018 +0200

	    tls: convert to generic sk_msg interface

So telling syzbot:

#syz fix: tls: convert to generic sk_msg interface

The issue was that described in this comment in tls_sw_sendmsg():

                /* Open records defined only if successfully copied, otherwise
                 * we would trim the sg but not reset the open record frags.
                 */
                tls_ctx->pending_open_record_frags = true;

Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.

- Eric

^ permalink raw reply

* Re: [PATCH] can: mark expected switch fall-throughs
From: Marc Kleine-Budde @ 2019-02-26  8:02 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Wolfgang Grandegger, David S. Miller,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20190129180612.GA28650@embeddedor>

On 1/29/19 7:06 PM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    if (n & PCAN_USB_ERROR_BUS_LIGHT) {
>       ^
> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
>   case CAN_STATE_ERROR_WARNING:
>   ^~~~
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> This patch is part of the ongoing efforts to enabling
> -Wimplicit-fallthrough.
> 
> Notice that in some cases spelling mistakes were fixed.
> In other cases, the /* fall through */ comment is placed
> at the bottom of the case statement, which is what GCC
> is expecting to find.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Added to linux-can-next.

Tnx
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

^ permalink raw reply

* [PATCH] net: phy: Micrel KSZ8061: link failure after cable connect
From: Rajasingh Thavamani @ 2019-02-26  8:15 UTC (permalink / raw)
  Cc: t.rajasingh, Rajasingh Thavamani, Andrew Lunn, Florian Fainelli,
	Heiner Kallweit, David S. Miller, netdev, linux-kernel

With Micrel KSZ8061 PHY, the link may occasionally not come up after
Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
sheet 80000688A.pdf descripes the problem and possible workarounds in
detail, see below.
The batch implements workaround 1, which permanently fixes the issue.

DESCRIPTION
Link-up may not occur properly when the Ethernet cable is initially
connected. This issue occurs more commonly when the cable is connected
slowly, but it may occur any time a cable is connected. This issue occurs
in the auto-negotiation circuit, and will not occur if auto-negotiation
is disabled (which requires that the two link partners be set to the
same speed and duplex).

END USER IMPLICATIONS
When this issue occurs, link is not established. Subsequent cable
plug/unplaug cycle will not correct the issue.

WORk AROUND
There are four approaches to work around this issue:
1. This issue can be prevented by setting bit 15 in MMD device address 1,
   register 2, prior to connecting the cable or prior to setting the
   Restart Auto-negotiation bit in register 0h. The MMD registers are
   accessed via the indirect access registers Dh and Eh, or via the Micrel
   EthUtil utility as shown here:
   . if using the EthUtil utility (usually with a Micrel KSZ8061
     Evaluation Board), type the following commands:
     > address 1
     > mmd 1
     > iw 2 b61a
   . Alternatively, write the following registers to write to the
     indirect MMD register:
     Write register Dh, data 0001h
     Write register Eh, data 0002h
     Write register Dh, data 4001h
     Write register Eh, data B61Ah
2. The issue can be avoided by disabling auto-negotiation in the KSZ8061,
   either by the strapping option, or by clearing bit 12 in register 0h.
   Care must be taken to ensure that the KSZ8061 and the link partner
   will link with the same speed and duplex. Note that the KSZ8061
   defaults to full-duplex when auto-negotiation is off, but other
   devices may default to half-duplex in the event of failed
   auto-negotiation.
3. The issue can be avoided by connecting the cable prior to powering-up
   or resetting the KSZ8061, and leaving it plugged in thereafter.
4. If the above measures are not taken and the problem occurs, link can
   be recovered by setting the Restart Auto-Negotiation bit in
   register 0h, or by resetting or power cycling the device. Reset may
   be either hardware reset or software reset (register 0h, bit 15).

PLAN
This errata will not be corrected in the future revision.

Signed-off-by: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
---
 drivers/net/phy/micrel.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index b1f959935f50..b7df0295a3ca 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -344,6 +344,17 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
 	return genphy_config_aneg(phydev);
 }
 
+static int ksz8061_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_DEVID1, 0xB61A);
+	if (ret)
+		return ret;
+
+	return kszphy_config_init(phydev);
+}
+
 static int ksz9021_load_values_from_of(struct phy_device *phydev,
 				       const struct device_node *of_node,
 				       u16 reg,
@@ -1040,7 +1051,7 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KSZ8061",
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.features	= PHY_BASIC_FEATURES,
-	.config_init	= kszphy_config_init,
+	.config_init	= ksz8061_config_init,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
 	.suspend	= genphy_suspend,
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH ethtool V2] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Michal Kubecek @ 2019-02-26  8:16 UTC (permalink / raw)
  To: Tariq Toukan
  Cc: John W. Linville, netdev, Eran Ben Elisha, alaa, Andrew Lunn,
	Aya Levin
In-Reply-To: <1551109474-12838-1-git-send-email-tariqt@mellanox.com>

On Mon, Feb 25, 2019 at 05:44:34PM +0200, Tariq Toukan wrote:
> From: Aya Levin <ayal@mellanox.com>
> 
> Introduce 50Gbps per lane link modes and 200Gbps speed, update print
> functions and initialization functions accordingly.
> In addition, update related man page accordingly.
> 
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> ---
>  ethtool-copy.h | 18 +++++++++++++++++-
>  ethtool.8.in   | 15 +++++++++++++++
>  ethtool.c      | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> V2:
> - Removed wrong and unneeded initialization of adv_bit.
> - Removed an addition in ethtool-copy.h that does not exist in kernel.

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

However, as these link modes are defined only in net-next tree at the
moment and won't get into mainline until the 5.1-rc1 merge window, maybe
this should be applied after ethtool 5.0 release (if there is one).

Michal

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Leslie Monis @ 2019-02-26  8:20 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem, netdev, Mohit P . Tahiliani, Dave Taht, Jamal Hadi Salim
In-Reply-To: <20190225163811.4a6b477b@shemminger-XPS-13-9360>

On Mon, Feb 25, 2019 at 04:38:11PM -0800, Stephen Hemminger wrote:
> On Tue, 26 Feb 2019 00:39:54 +0530
> Leslie Monis <lesliemonis@gmail.com> wrote:
> 
> > The current implementation of the PIE queuing discipline is according to the
> > IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> > [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> > However, a lot of necessary modifications and enhancements have been proposed
> > in RFC 8033, which have not yet been incorporated in the source code of Linux.
> > This patch series helps in achieving the same.
> > 
> > Performance tests carried out using Flent [https://flent.org/]
> > 
> > Changes from v2 to v3:
> >   - Used div_u64() instead of direct division after explicit type casting as
> >     recommended by David
> > 
> > Changes from v1 to v2:
> >   - Excluded the patch setting PIE dynamically active/inactive as the test
> >     results were unsatisfactory
> >   - Fixed a scaling issue when adding more auto-tuning cases which caused
> >     local variables to underflow
> >   - Changed the long if/else chain to a loop as suggested by Stephen
> >   - Changed the position of the accu_prob variable in the pie_vars
> >     structure as recommended by Stephen
> > 
> > Mohit P. Tahiliani (7):
> >   net: sched: pie: change value of QUEUE_THRESHOLD
> >   net: sched: pie: change default value of pie_params->target
> >   net: sched: pie: change default value of pie_params->tupdate
> >   net: sched: pie: change initial value of pie_vars->burst_time
> >   net: sched: pie: add more cases to auto-tune alpha and beta
> >   net: sched: pie: add derandomization mechanism
> >   net: sched: pie: update references
> > 
> >  include/uapi/linux/pkt_sched.h |   2 +-
> >  net/sched/sch_pie.c            | 107 ++++++++++++++++++++-------------
> >  2 files changed, 66 insertions(+), 43 deletions(-)
> 
> Are you concerned at all that changes to default values might change
> expected behavior of existing users?

Hi Stephen,

As Dave mentioned, the changes which we have made do not really change the
behaviour of the aqm drastically. Our performance tests show that these changes
improve performance without any side-effects. So existing users (if there are
any) should not be negatively affected in any way.

Leslie

^ permalink raw reply

* [PATCH] tcp: fix __tcp_transmit_skb's comment text
From: Geliang Tang @ 2019-02-26  8:41 UTC (permalink / raw)
  To: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: Geliang Tang, netdev, linux-kernel

The function name tcp_do_sendmsg has been renamed. But it still
appears in __tcp_transmit_skb's comment text. This patch changes
it to tcp_sendmsg_locked.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/ipv4/tcp_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index e72aa0ff5785..67a43b966b8a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1003,7 +1003,7 @@ static void tcp_update_skb_after_send(struct sock *sk, struct sk_buff *skb,
 }
 
 /* This routine actually transmits TCP packets queued in by
- * tcp_do_sendmsg().  This is used by both the initial
+ * tcp_sendmsg_locked().  This is used by both the initial
  * transmission and possible later retransmissions.
  * All SKB's seen here are completely headerless.  It is our
  * job to build the TCP header, and pass the packet down to
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 net] net: phy: phylink: fix uninitialized variable in phylink_get_mac_state
From: Russell King - ARM Linux admin @ 2019-02-26  8:43 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: David Miller, Andrew Lunn, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <6c710eec-a299-5239-dde1-097209e8868d@gmail.com>

On Tue, Feb 26, 2019 at 08:25:41AM +0100, Heiner Kallweit wrote:
> When debugging an issue I found implausible values in state->pause.
> Reason in that state->pause isn't initialized and later only single
> bits are changed. Also the struct itself isn't initialized in
> phylink_resolve(). So better initialize state->pause.

mac_link_state() is expected to always set this, but this is safer.

Maybe also set state->speed to SPEED_UNKNOWN, state->duplex to
DUPLEX_UNKNOWN and state->an_complete to zero?

> 
> v2:
> - use right function name in subject
> 
> Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phylink.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 59d175a5b..a9954c205 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -324,6 +324,7 @@ static int phylink_get_mac_state(struct phylink *pl, struct phylink_link_state *
>  	linkmode_zero(state->lp_advertising);
>  	state->interface = pl->link_config.interface;
>  	state->an_enabled = pl->link_config.an_enabled;
> +	state->pause = MLO_PAUSE_NONE;
>  	state->link = 1;
>  
>  	return pl->ops->mac_link_state(ndev, state);
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH][next] Bluetooth: mgmt: Use struct_size() helper
From: Marcel Holtmann @ 2019-02-26  8:48 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Johan Hedberg, David S. Miller, open list:BLUETOOTH DRIVERS,
	netdev, linux-kernel
In-Reply-To: <20190225191137.GA24383@embeddedor>

Hi Gustavo,

> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, change the following form:
> 
> sizeof(*rp) + (sizeof(rp->entry[0]) * count);
> 
> to :
> 
> struct_size(rp, entry, count)
> 
> Notice that, in this case, variable rp_len is not necessary, hence
> it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> net/bluetooth/mgmt.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: Jiri Pirko @ 2019-02-26  8:54 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190225190132.GA23478@embeddedor>

Mon, Feb 25, 2019 at 08:01:32PM CET, gustavo@embeddedor.com wrote:
>One of the more common cases of allocation size calculations is finding
>the size of a structure that has a zero-sized array at the end, along
>with memory for some number of elements for that array. For example:
>
>struct foo {
>    int stuff;
>    struct boo entry[];
>};
>
>size = sizeof(struct foo) + count * sizeof(struct boo);
>instance = kzalloc(size, GFP_KERNEL)
>
>Instead of leaving these open-coded and prone to type mistakes, we can
>now use the new struct_size() helper:
>
>instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
>
>Notice that, in this case, variable alloc_size is not necessary, hence
>it is removed.
>
>This code was detected with the help of Coccinelle.
>
>Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance
From: Jiri Pirko @ 2019-02-26  8:56 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-3-jakub.kicinski@netronome.com>

Tue, Feb 26, 2019 at 04:34:03AM CET, jakub.kicinski@netronome.com wrote:
>Instead of iterating over all devlink ports add a NDO which
>will return the devlink instance from the driver.
>
>v2: add the netdev_to_devlink() helper (Michal)
>v3: check that devlink has ops (Florian)
>v4: hold devlink_mutex (Jiri)
>
>Suggested-by: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: Sabrina Dubroca @ 2019-02-26  9:06 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: David Ahern, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <20190226034854.GT10051@dhcp-12-139.nay.redhat.com>

2019-02-26, 11:48:54 +0800, Hangbin Liu wrote:
> Hi David,
> On Mon, Feb 25, 2019 at 07:23:33PM -0700, David Ahern wrote:
> > On 2/25/19 7:17 PM, Hangbin Liu wrote:
> > > I also thought about this issue. Currently we didn't check the ipproto in both
> > > IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
> > > This looks don't make any serious problem. It's just a user mis-configuration,
> > > the kernel check the proto number and won't match normal IP/IPv6 headers.
> > > 
> > > But yes, we should make it more strict, do you think if I should add a new
> > > rtm_getroute_parse_ip6_proto() function, or just add a family parameter
> > > in previous function?
> > 
> > I see now. rtm_getroute_parse_ip_proto is used for ipv4 and ipv6. For v4
> > IPPROTO_ICMPV6 should not be allowed and for v6 IPPROTO_ICMP should
> > fail. You could a version argument to rtm_getroute_parse_ip_proto and
> > fail as needed.
> 
> Sorry I didn't get here. Do you mean add an IPv6 version of
> rtm_getroute_parse_ip_proto?

Add an argument to rtm_getroute_parse_ip_proto that tells what IP
version to use, and then handle IPPROTO_ICMP/IPPROTO_ICMPV6 depending
on that.

For example:

int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
				bool ipv6, struct netlink_ext_ack *extack)

And pass false from ipv4/true from ipv6.

-- 
Sabrina

^ permalink raw reply

* Re: [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jiri Pirko @ 2019-02-26  8:57 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-6-jakub.kicinski@netronome.com>

Tue, Feb 26, 2019 at 04:34:06AM CET, jakub.kicinski@netronome.com wrote:
>When ethtool is calling into devlink compat code make sure we have
>a reference on the netdevice on which the operation was invoked.
>
>v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
From: Jiri Pirko @ 2019-02-26  8:59 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-7-jakub.kicinski@netronome.com>

Tue, Feb 26, 2019 at 04:34:07AM CET, jakub.kicinski@netronome.com wrote:
>Commit 76726ccb7f46 ("devlink: add flash update command") and
>commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
>access devlink ops without NULL-checking. There is, however, no
>driver which would pass in NULL ops, so let's just make that
>a requirement. Remove the now unnecessary NULL-checking.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Thanks Kuba!

^ permalink raw reply

* Re: [PATCH v4 0/3] Add quirk for reading BD_ADDR from fwnode property
From: Marcel Holtmann @ 2019-02-26  9:09 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Johan Hedberg, David S. Miller, Loic Poulain, linux-bluetooth,
	linux-kernel, netdev, Balakrishna Godavarthi
In-Reply-To: <20190219200559.13079-1-mka@chromium.org>

Hi Matthias,

> On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> on the Bluetooth chip itself. One way to configure the address is
> through the device tree (patched in by the bootloader). The btqcomsmd
> driver is an example, it can read the address from the DT property
> 'local-bd-address'.
> 
> To avoid redundant open-coded reading of 'local-bd-address' and error
> handling this series adds the quirk HCI_QUIRK_USE_BDADDR_PROPERTY to
> retrieve the BD address of a device from the DT and adapts the
> btqcomsmd and hci_qca drivers to use this quirk.
> 
> Matthias Kaehlcke (3):
>  Bluetooth: Add quirk for reading BD_ADDR from fwnode property
>  Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
>  Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990
> 
> drivers/bluetooth/btqcomsmd.c | 31 +++----------------------
> drivers/bluetooth/hci_qca.c   |  1 +
> include/net/bluetooth/hci.h   | 12 ++++++++++
> net/bluetooth/hci_core.c      | 43 +++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c          |  6 +++--
> 5 files changed, 63 insertions(+), 30 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* RE: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx buffer
From: Ioana Ciornei @ 2019-02-26  9:14 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Ioana Ciocoi Radulescu
  Cc: Ilias Apalodimas, netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <VI1PR0402MB28001EBE94356BFF7C250C86E0690@VI1PR0402MB2800.eurprd04.prod.outlook.com>


> Subject: RE: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx buffer
> 
> > Subject: Re: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx
> > buffer
> >
> >
> > On Wed, 6 Feb 2019 15:36:33 +0000 Ioana Ciocoi Radulescu
> > <ruxandra.radulescu@nxp.com> wrote:
> >
> > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >
> > > > Can you share any results on XDP (XDP_DROP is usually useful for
> > > > the hardware capabilities).
> > >
> > > XDP numbers are pretty much the same as before this patch:
> > >
> > > On a LS2088A with A72 cores @2GHz (numbers in Mpps):
> > > 				1core		8cores
> > > -------------------------------------------------------------------------
> > > XDP_DROP (no touching data)	5.37		29.6 (linerate)
> > > XDP_DROP (xdp1 sample)	3.14		24.22
> >
> > It is interesting/problematic to see that the cost of touching the
> > data is so high 5.37Mpps -> 3.14Mpps.  The Intel CPUs have solved this
> > in hardware with DDIO, which delivers frame in L3-cache. I have some
> > ideas on how to improve this on ARM (or CPUs without DDIO).  I've
> > previous implemented this as RFC on mlx4 tested on a CPU without DDIO,
> > with great success 10mpps -> 20Mpps (but it was shutdown, as newer
> > Intel HW solved the issue).  The basic idea is to have an array of
> > frames, that you start an L2/L3-prefetch on, before going "back" and
> > process them for XDP or netstack. (p.s. this is the same DPDK does)
> 
> Thanks for the hint. We are currently investigating what our options are.
> We'll come back with a more detailed answer once we figure out.
> 

Hi,

We have enabled cache stashing of frame data in our solution and sent the patches upstream: https://lkml.org/lkml/2019/2/23/41.

Some results with the patches applied on top of the net-next tree are below.
LS2088A with A72 cores @2GHz (numbers in Mpps):

 			                		1core		8cores
 --------------------------------------------------------------------------------------------------------
 XDP_DROP (no touching data)			5.37		29.6 (linerate)
 XDP_DROP (xdp1 sample, no stashing)		3.28		24.2
 XDP_DROP (xdp1 sample, stashing)		4.64		26.85

Thanks a lot for your hint.

Ioana C

^ permalink raw reply

* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Nicolas.Ferre @ 2019-02-26  9:23 UTC (permalink / raw)
  To: f.fainelli, pthombar, davem, netdev, andrew, hkallweit1,
	linux-kernel, rafalc, piotrs, jank, Claudiu.Beznea
In-Reply-To: <65c93555-f0d2-1139-a4fd-5c6a80acd9bf@gmail.com>

$subject should begin with "net: macb: "

Parshuram,

Sorry but NACK on the series.

David,

This patch series seem pretty intrusive, so I would like that you wait 
for my explicit ACK before applying even next versions of it.

More comments below...


On 25/02/2019 at 18:21, Florian Fainelli wrote:
> On 2/25/19 1:11 AM, Parshuram Raju Thombare wrote:
>>> Le 2/22/19 à 12:12 PM, Parshuram Thombare a écrit :
>>>> This patch add support for PCS (for SGMII interface) and 2.5Gbps MAC
>>>> in Cadence ethernet controller driver.
>>>
>>> At a high level you don't seem to be making use of PHYLINK so which 2.5Gbps
>>> interfaces do you actually support?
>>>
>>
>> New ethernet controller have MAC which support 2.5G speed.
>> Also there is addition of PCS and SGMII interface.
> 
> I should have asked this more clearly: have you tested with SFP modules
> for instance? If you want to be able to reliably support 2500baseT
> and/or 2500baseX with hot plugging of such modules, you need to
> implement PHYLINK for that network driver, there is no other way around.
> 
>>
>>>>
>>>> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
>>>> ---
>>>

[..]

>>>>
>>>> -	switch (speed) {
>>>> -	case SPEED_10:
>>>> +	if (interface == PHY_INTERFACE_MODE_GMII ||
>>>> +	    interface == PHY_INTERFACE_MODE_MII) {
>>>> +		switch (speed) {
>>>> +		case SPEED_10:>  		rate = 2500000;
>>>
>>> You need to add one tab to align rate and break.
>>
>> Do you mean a tab each for rate and break lines ?
>> All switch statements are aligned at a tab.  I am not sure how does case and rate got on same line.
> 
> It should look like this:
> 
> switch (cond) {
> case cond1:
> 	do_something();
> 	break;
> 
> etc.

Read the coding style documentation, everything is well explained there.


>>>>   		break;
>>>> -	case SPEED_100:
>>>> +		case SPEED_100:
>>>>   		rate = 25000000;
>>>>   		break;
>>>> -	case SPEED_1000:
>>>> +		case SPEED_1000:
>>>>   		rate = 125000000;
>>>>   		break;
>>>> -	default:
>>>> +		default:
>>>> +		return;
>>>> +		}
>>>> +	} else if (interface == PHY_INTERFACE_MODE_SGMII) {
>>>> +		switch (speed) {
>>>> +		case SPEED_10:
>>>> +		rate = 1250000;
>>>> +		break;
>>>> +		case SPEED_100:
>>>> +		rate = 12500000;
>>>> +		break;
>>>> +		case SPEED_1000:
>>>> +		rate = 125000000;
>>>> +		break;
>>>> +		case SPEED_2500:
>>>> +		rate = 312500000;
>>>> +		break;
>>>> +		default:
>>>> +		return;
>>>
>>> The indentation is broken here and you can greatly simplify this with a simple
>>> function that returns speed * 1250 and does an initial check for unsupported
>>> speeds.
>>>
>>
>> I ran checkpatch.pl and all indentation issues were cleared. But I think having function
>> is better option, I will make that change.
>>
>>>> +		}
>>>> +	} else {
>>>>   		return;
>>>>   	}

[..]

>>>> int macb_mii_probe(struct net_device *dev)
>>>>   	}
>>>>
>>>>   	/* mask with MAC supported features */
>>>> -	if (macb_is_gem(bp) && bp->caps &
>>> MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>>>> -		phy_set_max_speed(phydev, SPEED_1000);
>>>> -	else
>>>> -		phy_set_max_speed(phydev, SPEED_100);
>>>> +	if (macb_is_gem(bp)) {
>>>
>>> You have changed the previous logic that also checked for
>>> MACB_CAPS_GIGABIT_MODE_AVAILABLE, why?
>>
>> My understanding is all GEM (ID >= 0x2) support GIGABIT mode.
>> Was there any other reason for this check ?

We use (ID >= 0x2) and only 10/100 mode on sama5d4/sama5d2 for more than 
5 years, as seen in their respective struct macb_config, weren't you aware?

> Well, if anyone would know, it would be you, I don't work for Cadence
> nor have access to the internal IP documentation.

I agree with Florian, and what you modify makes me feel that the rest of 
the patch might break my use of the Cadence IP in my products. That's 
not a good sign, to say the least.

I'm expecting that Cadence don't break software used by their customers 
on products already deployed on the field.
For next series, I would like that you report that you actually tested 
your changes on older IP revisions and that non-regression tests are 
passed successfully for some of the long time users of this IP (namely 
Microchip/Atmel and Xilinx products).



>>>> +		linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>>>> +		if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>>>> +
>>> 	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>>>> +					 phydev->supported);
>>>> +	} else {
>>>> +		linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>>>> +	}
>>>> +
>>>> +	linkmode_copy(phydev->advertising, phydev->supported);
>>>>
>>>>   	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>>>>   		phy_remove_link_mode(phydev,
>>>> @@ -2217,8 +2267,6 @@ static void macb_init_hw(struct macb *bp)
>>>>   	macb_set_hwaddr(bp);
>>>>
>>>>   	config = macb_mdc_clk_div(bp);
>>>> -	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>>>> -		config |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
>>>>   	config |= MACB_BF(RBOF, NET_IP_ALIGN);	/* Make eth data
>>> aligned */
>>>>   	config |= MACB_BIT(PAE);		/* PAuse Enable */
>>>>   	config |= MACB_BIT(DRFCS);		/* Discard Rx FCS */
>>>> @@ -3255,6 +3303,23 @@ static void macb_configure_caps(struct macb *bp,
>>>>   		dcfg = gem_readl(bp, DCFG1);
>>>>   		if (GEM_BFEXT(IRQCOR, dcfg) == 0)
>>>>   			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>>>> +		if (GEM_BFEXT(NO_PCS, dcfg) == 0)
>>>> +			bp->caps |= MACB_CAPS_PCS;
>>>> +		switch (MACB_BFEXT(IDNUM, macb_readl(bp, MID))) {
>>>> +		case MACB_GEM7016_IDNUM:
>>>> +		case MACB_GEM7017_IDNUM:
>>>> +		case MACB_GEM7017A_IDNUM:
>>>> +		case MACB_GEM7020_IDNUM:
>>>> +		case MACB_GEM7021_IDNUM:
>>>> +		case MACB_GEM7021A_IDNUM:
>>>> +		case MACB_GEM7022_IDNUM:
>>>> +		if (bp->caps & MACB_CAPS_PCS)
>>>> +			bp->caps |= MACB_CAPS_TWO_PT_FIVE_GIG_SPEED;
>>>> +		break;
>>>> +
>>>> +		default:
>>>> +		break;
>>>> +		}
>>>>   		dcfg = gem_readl(bp, DCFG2);
>>>>   		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF)))
>>> == 0)
>>>>   			bp->caps |= MACB_CAPS_FIFO_MODE;
>>>> @@ -4110,7 +4175,28 @@ static int macb_probe(struct platform_device
>>> *pdev)
>>>>   		else
>>>>   			bp->phy_interface = PHY_INTERFACE_MODE_MII;
>>>>   	} else {
>>>> +		switch (err) {
>>>> +		case PHY_INTERFACE_MODE_SGMII:
>>>> +		if (bp->caps & MACB_CAPS_PCS) {
>>>> +			bp->phy_interface = PHY_INTERFACE_MODE_SGMII;
>>>> +			break;
>>>> +		}
>>>
>>> If SGMII was selected on a version of the IP that does not support it, then falling
>>> back to GMII or MII does not sound correct, this is a hard error that must be
>>> handled as such.
>>> --
>>> Florian
>>
>> My intention was to continue (instead of failing) with whatever functionality is available.
>> Can we have some error message and continue with what is available ?
> 
> This is probably not going to help anyone, imagine you incorrectly
> specified a 'phy-mode' property in the Device Tree and you have to dig
> into the driver in the function that sets the clock rate to find out
> what you just defaulted to 1Gbits/GMII for instance. This is not user
> friendly at all and will increase the support burden on your end as
> well, if SGMII is specified and the IP does not support it, detect it
> and return an error, how that propagates, either as a probe failure or
> the inability to open the network device, your call.
> 

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH net] tcp: repaired skbs must init their tso_segs
From: Andrei Vagin @ 2019-02-26  9:23 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Eric Dumazet, Soheil Hassas Yeganeh,
	Neal Cardwell, Yuchung Cheng, syzbot, Andrey Vagin
In-Reply-To: <20190223235151.168283-1-edumazet@google.com>

On Sat, Feb 23, 2019 at 03:51:51PM -0800, Eric Dumazet wrote:
> syzbot reported a WARN_ON(!tcp_skb_pcount(skb))
> in tcp_send_loss_probe() [1]
> 
> This was caused by TCP_REPAIR sent skbs that inadvertenly
> were missing a call to tcp_init_tso_segs()
> 
> [1]
> WARNING: CPU: 1 PID: 0 at net/ipv4/tcp_output.c:2534 tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.0.0-rc7+ #77
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  <IRQ>
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
>  panic+0x2cb/0x65c kernel/panic.c:214
>  __warn.cold+0x20/0x45 kernel/panic.c:571
>  report_bug+0x263/0x2b0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
>  fixup_bug arch/x86/kernel/traps.c:173 [inline]
>  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>  do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
> RIP: 0010:tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Code: 88 fc ff ff 4c 89 ef e8 ed 75 c8 fb e9 c8 fc ff ff e8 43 76 c8 fb e9 63 fd ff ff e8 d9 75 c8 fb e9 94 f9 ff ff e8 bf 03 91 fb <0f> 0b e9 7d fa ff ff e8 b3 03 91 fb 0f b6 1d 37 43 7a 03 31 ff 89
> RSP: 0018:ffff8880ae907c60 EFLAGS: 00010206
> RAX: ffff8880a989c340 RBX: 0000000000000000 RCX: ffffffff85dedbdb
> RDX: 0000000000000100 RSI: ffffffff85dee0b1 RDI: 0000000000000005
> RBP: ffff8880ae907c90 R08: ffff8880a989c340 R09: ffffed10147d1ae1
> R10: ffffed10147d1ae0 R11: ffff8880a3e8d703 R12: ffff888091b90040
> R13: ffff8880a3e8d540 R14: 0000000000008000 R15: ffff888091b90860
>  tcp_write_timer_handler+0x5c0/0x8a0 net/ipv4/tcp_timer.c:583
>  tcp_write_timer+0x10e/0x1d0 net/ipv4/tcp_timer.c:607
>  call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
>  expire_timers kernel/time/timer.c:1362 [inline]
>  __run_timers kernel/time/timer.c:1681 [inline]
>  __run_timers kernel/time/timer.c:1649 [inline]
>  run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
>  __do_softirq+0x266/0x95a kernel/softirq.c:292
>  invoke_softirq kernel/softirq.c:373 [inline]
>  irq_exit+0x180/0x1d0 kernel/softirq.c:413
>  exiting_irq arch/x86/include/asm/apic.h:536 [inline]
>  smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
>  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
>  </IRQ>
> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 0c a1 fa 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 48 0c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
> RSP: 0018:ffff8880a98afd78 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
> RAX: 1ffffffff1125061 RBX: ffff8880a989c340 RCX: 0000000000000000
> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffff8880a989cbbc
> RBP: ffff8880a98afda8 R08: ffff8880a989c340 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
> R13: ffffffff889282f8 R14: 0000000000000001 R15: 0000000000000000
>  arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
>  default_idle_call+0x36/0x90 kernel/sched/idle.c:93
>  cpuidle_idle_call kernel/sched/idle.c:153 [inline]
>  do_idle+0x386/0x570 kernel/sched/idle.c:262
>  cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
>  start_secondary+0x404/0x5c0 arch/x86/kernel/smpboot.c:271
>  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
> 

Thank you Eric. I saw a few test fails when tcp_peek_sndq()
returned more data than we expected. I have executed the test with this
fix in a loop and it works without any problem. Without this fix, it
fails after a few iteration.

https://github.com/checkpoint-restore/criu/issues/622

> Fixes: 79861919b889 ("tcp: fix TCP_REPAIR xmit queue setup")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Andrey Vagin <avagin@openvz.org>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
>  net/ipv4/tcp_output.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 730bc44dbad9363814705b28c2f91a2253d91207..ccc78f3a4b60d3012430488bdfbcfc5122ff8627 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2347,6 +2347,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
>  			/* "skb_mstamp_ns" is used as a start point for the retransmit timer */
>  			skb->skb_mstamp_ns = tp->tcp_wstamp_ns = tp->tcp_clock_cache;
>  			list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
> +			tcp_init_tso_segs(skb, mss_now);
>  			goto repair; /* Skip network transmission */
>  		}
>  
> -- 
> 2.21.0.rc0.258.g878e2cd30e-goog
> 

^ permalink raw reply

* [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Julius Niedworok @ 2019-02-26  9:40 UTC (permalink / raw)
  To: linux-wireless
  Cc: julius.n, ga58taw, david, nc, David S. Miller, Johannes Berg,
	Edward Cree, Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
	Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
	linux-kernel

At Technical University of Munich we use MAC 802.11 TX status frames to
perform several measurements in MAC 802.11 setups.

With ath based drivers this was possible until commit d94a461d7a7df6
("ath9k: use ieee80211_tx_status_noskb where possible") as the driver
ignored the IEEE80211_TX_CTL_REQ_TX_STATUS flag and always delivered
TX status frames. Since this commit, this behavior was changed and the
driver now adheres to IEEE80211_TX_CTL_REQ_TX_STATUS.

Due to performance reasons, IEEE80211_TX_CTL_REQ_TX_STATUS is not set for
data frames from interfaces in managed mode. Hence, frames that are sent
from a managed mode interface do never deliver TX status frames. This
remains true even if a monitor mode interface (e.g. a measurement
interface) is added to the same wireless hardware device. Thus, there is
no possibility for receiving TX status frames for frames sent on an
interface in managed mode if the driver adheres to
IEEE80211_TX_CTL_REQ_TX_STATUS.

In order to force delivery of TX status frames for research and debugging
purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag
is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled
in all packets sent from that interface. IFF_ECHO can be set via
/sys/class/net/<dev>/flags. The default is disabled.

Co-developed-by: Charlie Groh <ga58taw@mytum.de>
Signed-off-by: Charlie Groh <ga58taw@mytum.de>
Signed-off-by: Julius Niedworok <julius.n@gmx.net>
---
 net/core/dev.c    | 8 ++++++++
 net/mac80211/tx.c | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8e276e0..076b556 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7543,6 +7543,14 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags,
 			       IFF_AUTOMEDIA)) |
 		     (dev->flags & (IFF_UP | IFF_VOLATILE | IFF_PROMISC |
 				    IFF_ALLMULTI));
+	/*
+	 *	Force TX status frames on ieee80211 devices.
+	 *	Since IFF_ECHO is used by CAN devices for a different
+	 *	purpose, we must check dev->ieee80211_ptr.
+	 */
+
+	if (dev->ieee80211_ptr)
+		dev->flags = (dev->flags & ~IFF_ECHO) | (flags & IFF_ECHO);
 
 	/*
 	 *	Load in the correct multicast list now the flags have changed.
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 928f13a..2a02f66 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2463,6 +2463,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 	if (IS_ERR(sta))
 		sta = NULL;
 
+	if (sdata->dev->flags & IFF_ECHO)
+		info_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+
 	/* convert Ethernet header to proper 802.11 header (based on
 	 * operation mode) */
 	ethertype = (skb->data[12] << 8) | skb->data[13];
@@ -3468,6 +3471,9 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
 		      (tid_tx ? IEEE80211_TX_CTL_AMPDU : 0);
 	info->control.flags = IEEE80211_TX_CTRL_FAST_XMIT;
 
+	if (sdata->dev->flags & IFF_ECHO)
+		info->flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+
 	if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
 		tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
 		*ieee80211_get_qos_ctl(hdr) = tid;
-- 
2.10.1 (Apple Git-78)


^ permalink raw reply related

* [PATCH net-next 1/1] net: sched: act_csum: Fix csum calc for tagged packets
From: Eli Britstein @ 2019-02-26  9:57 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, Jamal Hadi Salim, Cong Wang, Roi Dayan, Maor Dickman,
	Eli Britstein

The csum calculation is different for IPv4/6. For VLAN packets,
tc_skb_protocol returns the VLAN protocol rather than the packet's one
(e.g. IPv4/6), so csum is not calculated. Furthermore, VLAN may not be
stripped so csum is not calculated in this case too. Calculate the
csum for those cases.

Fixes: d8b9605d2697 ("net: sched: fix skb->protocol use in case of accelerated vlan path")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/act_csum.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 945fb34ae721..c79aca29505e 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -559,8 +559,11 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
 			struct tcf_result *res)
 {
 	struct tcf_csum *p = to_tcf_csum(a);
+	bool orig_vlan_tag_present = false;
+	unsigned int vlan_hdr_count = 0;
 	struct tcf_csum_params *params;
 	u32 update_flags;
+	__be16 protocol;
 	int action;
 
 	params = rcu_dereference_bh(p->params);
@@ -573,7 +576,9 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
 		goto drop;
 
 	update_flags = params->update_flags;
-	switch (tc_skb_protocol(skb)) {
+	protocol = tc_skb_protocol(skb);
+again:
+	switch (protocol) {
 	case cpu_to_be16(ETH_P_IP):
 		if (!tcf_csum_ipv4(skb, update_flags))
 			goto drop;
@@ -582,13 +587,35 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
 		if (!tcf_csum_ipv6(skb, update_flags))
 			goto drop;
 		break;
+	case cpu_to_be16(ETH_P_8021AD): /* fall through */
+	case cpu_to_be16(ETH_P_8021Q):
+		if (skb_vlan_tag_present(skb) && !orig_vlan_tag_present) {
+			protocol = skb->protocol;
+			orig_vlan_tag_present = true;
+		} else {
+			struct vlan_hdr *vlan = (struct vlan_hdr *)skb->data;
+
+			protocol = vlan->h_vlan_encapsulated_proto;
+			skb_pull(skb, VLAN_HLEN);
+			skb_reset_network_header(skb);
+			vlan_hdr_count++;
+		}
+		goto again;
+	}
+
+out:
+	/* Restore the skb for the pulled VLAN tags */
+	while (vlan_hdr_count--) {
+		skb_push(skb, VLAN_HLEN);
+		skb_reset_network_header(skb);
 	}
 
 	return action;
 
 drop:
 	qstats_drop_inc(this_cpu_ptr(p->common.cpu_qstats));
-	return TC_ACT_SHOT;
+	action = TC_ACT_SHOT;
+	goto out;
 }
 
 static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
-- 
2.17.2


^ permalink raw reply related

* [PATCH] net: sched: pie: fix mistake in reference link
From: Leslie Monis @ 2019-02-26 10:23 UTC (permalink / raw)
  To: davem; +Cc: netdev, Leslie Monis

Fix the incorrect reference link to RFC 8033

Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index f8314a14a256..4c0670b6aec1 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,7 +17,7 @@
  * University of Oslo, Norway.
  *
  * References:
- * RFC 8033: https://tools.ietf.org/html/rfc8034
+ * RFC 8033: https://tools.ietf.org/html/rfc8033
  */
 
 #include <linux/module.h>
-- 
2.17.1


^ permalink raw reply related

* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Anand Moon @ 2019-02-26 10:25 UTC (permalink / raw)
  To: Martin Blumenstingl, Bartosz Golaszewski; +Cc: linux-amlogic, netdev
In-Reply-To: <CANAwSgR=PXtfMJWm_pOLYrV2imKpuw2FQOLT_J73tuu-q7WHww@mail.gmail.com>

Hi Martin,

On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
>
> hi Martin,
>
> +Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > I have seen Anand's your question in [0]:
> > > only issue is I have is the each time their is random MAC address so I
> > > get new IP from dhcp server.
> > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> >
> > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > pass it (via the .dtb) to the kernel.
> > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > for and example.
> >
> > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > controllers: [2] (please note that the function implementing this
> > binding was recently renamed: [3])
> > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > based binding yet
> >
> > Anand, if you want to work on this: feel free to do so!
> > I have the SDHC MMC driver and a discussion about the power-domain
> > drivers on my TODO-list, so I'm pretty busy at the moment.
> >
> >
> > Regards
> > Martin
> >
>
> Thanks for your inputs :) 8)
>
> After enable CONFIG_MESON_MX_EFUSE and added the alias
>

As far as I can tell this nvmem consist of field.
 Board ID
 MAC address
 Serial Number
 UID

I feel efues value in nvmem are read with following offset just for testing.
but it also need some driver changes to read from secure memory
by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> On Odroid C1+
> # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> *
> 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000    *00:1e:06:10:dc:ad*
> mac address from nvmem
> 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> 0000200
>
arch/arm/boot/dts/meson8b.dtsi
@@ -360,6 +360,18 @@
        compatible = "amlogic,meson8b-efuse";
        clocks = <&clkc CLKID_EFUSE>;
        clock-names = "core";
+
+       board_sn: board-sn@0000000 {
+               reg = <0x0000000 0x10>;
+       };
+
+       eth_mac: eth-mac@00001b4 {
+               reg = <0x00001b4 0x6>;
+       };
+
+       board_sno: board-sno@00001c0 {
+               reg = <0x00001c0 0x30>;
+       };

This is what I am looking into.
If you have some input please share.

>
> Odroid C2:
> I could clearly observe the same.
>
> [root@archl-c2l alarm]# hexdump /sys/devices/platform/efuse/efuse0/nvmem
> 0000000 0000 0000 0000 0000 0000 0000 0000 0000
> 0000010 0000 0000 4b48 3243 3331 3532 4434 4346
> 0000020 4145 4146 0000 0000 0000 0000 0000 0000
> 0000030 0000 0000 1e00 3306 7a37 0000 0000 0000     *00:1e:06:33:37:7a*
> 0000040 0000 0000 0000 4b48 3243 3132 3631 3430
> 0000050 3834 3932 3631 3837 3937 3466 6233 6366
> 0000060 357c c7ec cc98 0f6d 65b7 92cf 000b 0a00
> 0000070 0000 0000 0000 0000 0000 0000 0000 0000
> 0000080 0014 0000 0000 0000 0000 0000 0000 0000
> 0000090 0000 0000 0000 0000 0000 0000 0000 0000
> *
       efuse: efuse {
                compatible = "amlogic,meson-gx-efuse",
"amlogic,meson-gxbb-efuse";
                #address-cells = <1>;
                #size-cells = <1>;
                read-only;

                sn: sn@14 {
                        reg = <0x14 0x10>;
                };

                eth_mac: eth_mac@34 {
                        reg = <0x34 0x10>;
                };

                bid: bid@46 {
                        reg = <0x46 0x30>;
                };
        };

> >
> > [0] http://lists.infradead.org/pipermail/linux-amlogic/2019-February/010464.html
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7c36209c46c4d162202b65eed2e66962ad8c3c1
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9217e566bdee4583d0a9ea4879c8f5e004886eac
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=afa64a72b862a7a9d04f8d07fba632eaf06b23f8
>
> Yes thanks for the inputs.
>
> I see c85efcc60a892210aa10688d5de1f997d5cad799 ("ARM: davinci:
> da830-evm: use cell nvmem lookup for mac address")
> I do not know what this changes will help get the mac address from nvmem region.
>
> If some body could share some inputs it will be better for me to debug.
>
> Best Regards
> -Anand

Best Regards
-Anand

^ permalink raw reply

* Re: [PATCH V2 3/8] dt-bindings: net: stmmac: add phys config properties
From: Christophe ROULLIER @ 2019-02-26 10:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: davem@davemloft.net, joabreu@synopsys.com, mark.rutland@arm.com,
	mcoquelin.stm32@gmail.com, Alexandre TORGUE, Peppe CAVALLARO,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
	andrew@lunn.ch
In-Reply-To: <20190223001651.GA22381@bogus>

On 2/23/19 1:16 AM, Rob Herring wrote:
> On Fri, Feb 22, 2019 at 09:28:04AM +0100, Christophe Roullier wrote:
>> Add properties to support all Phy config
>>   PHY_MODE	(MII,GMII, RMII, RGMII) and in normal, PHY wo crystal (25Mhz),
>>   PHY wo crystal (50Mhz), No 125Mhz from PHY config.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   Documentation/devicetree/bindings/net/stm32-dwmac.txt | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> index 1341012..f42dc68 100644
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> @@ -24,9 +24,9 @@ Required properties:
>>   	       encompases the glue register, and the offset of the control register.
>>   
>>   Optional properties:
>> -- clock-names:     For MPU family "mac-clk-ck" for PHY without quartz
>> -- st,int-phyclk (boolean) :  valid only where PHY do not have quartz and need to be clock
>> -	           by RCC
> 
> You can't just remove properties.

There is no risk to remove/rename these 2 properties, because it is 
specific board which is never deployed.
With new properties (renaming clock (eth-ck) + st,eth_clk_sel and 
st,eth_ref_clk_sel, we are managed all kind of specific boards stm32mp1
So no risk of backward compatible.

> 
>> +- clock-names:     For MPU family "eth-ck" for PHY without quartz
>> +- st,eth_clk_sel (boolean) : set this property in RGMII PHY when you do not want use 125Mhz
>> +- st,eth_ref_clk_sel (boolean) :  set this property in RMII mode when you have PHY without crystal 50MHz
> 
> s/_/-/
> 
> 'sel' I assume is short for select, but the naming here and description
> don't really tell me what I'm getting.
> 

Ok, Rob, I will update with your recommendations

> Rob
> 

^ permalink raw reply

* Re: [PATCH] net: sched: pie: fix mistake in reference link
From: Leslie Monis @ 2019-02-26 10:34 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <20190226102331.532-1-lesliemonis@gmail.com>

On Tue, Feb 26, 2019 at 03:53:31PM +0530, Leslie Monis wrote:
> Fix the incorrect reference link to RFC 8033
> 
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
> ---
>  net/sched/sch_pie.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index f8314a14a256..4c0670b6aec1 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -17,7 +17,7 @@
>   * University of Oslo, Norway.
>   *
>   * References:
> - * RFC 8033: https://tools.ietf.org/html/rfc8034
> + * RFC 8033: https://tools.ietf.org/html/rfc8033
>   */
>  
>  #include <linux/module.h>
> -- 
> 2.17.1
> 

I apologize. I forgot to add net-next in the subject prefix.

Leslie

^ permalink raw reply

* [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: Michael Chan @ 2019-02-26 10:56 UTC (permalink / raw)
  To: davem, maheshb, edumazet; +Cc: dja, netdev

There have been reports of oversize UDP packets being sent to the
driver to be transmitted, causing error conditions.  The issue is
likely caused by the dst of the SKB switching between 'lo' with
64K MTU and the hardware device with a smaller MTU.  Patches are
being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
issue.

Separately, we should add a length check in validate_xmit_skb()
to drop these oversize packets before they reach the driver.
This patch only validates non-TSO packets.  Complete validation
of segmented TSO packet size will probably be too slow.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 net/core/dev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5d03889..50c5174 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3373,6 +3373,13 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 		}
 	}
 
+	if (!skb_is_gso(skb) &&
+	    skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)) {
+		net_warn_ratelimited("%s(): Dropping %d bytes oversize skb.\n",
+				     __func__, skb->len);
+		goto out_kfree_skb;
+	}
+
 	skb = validate_xmit_xfrm(skb, features, again);
 
 	return skb;
-- 
2.5.1


^ permalink raw reply related

* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-26 11:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jesper Dangaard Brouer, David Miller, netdev, Daniel Borkmann,
	Alexei Starovoitov
In-Reply-To: <20190225104757.75b622c9@cakuba.netronome.com>

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

> On Sat, 23 Feb 2019 13:11:02 +0100, Toke Høiland-Jørgensen wrote:
>> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>> > On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski wrote:
>> >> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:  
>> >> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:    
>> >> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:      
>> > [...]  
>> >> > >
>> >> > > BPF programs don't obey by netns boundaries.  The fact the program is
>> >> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> >> > > Meaning if any program is using the redirect map you may need a secret
>> >> > > map in every ns.. no?      
>> >> > 
>> >> > Ah, yes, good point. Totally didn't think about the fact that load and
>> >> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> >> > alloc_default_map() to the point where the program is attached to an
>> >> > interface, then...    
>> >> 
>> >> Possibly.. and you also need to handle the case where interface with a
>> >> program attached is moved, no?  
>> 
>> Yup, alloc on attach was easy enough; the moving turns out to be the
>> tricky part :)
>> 
>> > True, we need to handle if e.g. a veth gets an XDP program attached and
>> > then is moved into a network namespace (as I've already explained to
>> > Toke in a meeting).  
>> 
>> Yeah, I had somehow convinced myself that the XDP program was being
>> removed when the interface was being torn down before moving between
>> namespaces. Jesper pointed out that this was not in fact the case... :P
>> 
>> > I'm still not sure how to handle this...  
>> 
>> There are a couple of options, I think. At least:
>> 
>> 1. Maintain a flag on struct net_device indicating that this device
>>    needs the redirect map allocated, and react to that when interfaces
>>    are being moved.
>> 
>> 2. Lookup the BPF program by ID (which we can get from the driver) on
>>    move, and react to the program flag.
>> 
>> 3. Keep the allocation on program load, but allocate maps for all active
>>    namespaces (which would probably need a refcnt mechanism to
>>    deallocate things again).
>> 
>> I think I'm leaning towards #2; possibly combined with a refcnt so we
>> can actually deallocate the map in the root namespace when it's not
>> needed anymore.
>
> Okay.. what about tail calls? I think #3 is most reasonable
> complexity- -wise, or some mix of #2 and #3 - cnt the programs with
> legacy redirects, and then allocate the resources if cnt && name space
> has any XDP program attached.

Yeah, I have that more or less working; except I forgot about tail
calls, but that should not be too difficult to fix.

> Can users really not be told to just use the correct helper? ;)

Experience would suggest not; users tend to use the simplest API that
gets their job done. And then wonder why they don't get the nice
performance numbers they were "promised". And, well, I tend to agree
that it's not terribly friendly to just go "use this other more
complicated API if you want proper performance". If we really mean that,
then we should formally deprecate xdp_redirect() as an API, IMO :)

-Toke

^ 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