* Re: [PATCH] net: dsa: sja1105: Fix missing unlock on error in sk_buff()
From: Florian Fainelli @ 2019-07-18 17:24 UTC (permalink / raw)
To: Wei Yongjun, Andrew Lunn, Vivien Didelot, Vladimir Oltean
Cc: netdev, kernel-janitors
In-Reply-To: <20190717062956.127446-1-weiyongjun1@huawei.com>
On 7/16/19 11:29 PM, Wei Yongjun wrote:
> Add the missing unlock before return from function sk_buff()
> in the error handling case.
>
> Fixes: f3097be21bf1 ("net: dsa: sja1105: Add a state machine for RX timestamping")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net] be2net: Synchronize be_update_queues with dev_watchdog
From: Florian Fainelli @ 2019-07-18 17:23 UTC (permalink / raw)
To: Benjamin Poirier, David Miller
Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
Firo Yang, Saeed Mahameed, netdev
In-Reply-To: <20190718014218.16610-1-bpoirier@suse.com>
On 7/17/19 6:42 PM, Benjamin Poirier wrote:
> As pointed out by Firo Yang, a netdev tx timeout may trigger just before an
> ethtool set_channels operation is started. be_tx_timeout(), which dumps
> some queue structures, is not written to run concurrently with
> be_update_queues(), which frees/allocates those queues structures. Add some
> synchronization between the two.
>
> Message-id: <CH2PR18MB31898E033896F9760D36BFF288C90@CH2PR18MB3189.namprd18.prod.outlook.com>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Would not moving the netif_tx_disable() in be_close() further up in the
function resolve that problem as well?
--
Florian
^ permalink raw reply
* Re: [PATCH] net: fec: generate warning when using deprecated phy reset
From: Sven Van Asbroeck @ 2019-07-18 17:21 UTC (permalink / raw)
To: Fabio Estevam
Cc: Lucas Stach, Fugang Duan, David S . Miller, netdev, linux-kernel
In-Reply-To: <CAOMZO5A_BuWMr1n_fFv4veyaXdcfjxO+9nFAgGfCrmAhNmzV5g@mail.gmail.com>
Lucas, Fabio,
On Thu, Jul 18, 2019 at 12:52 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > Not really a fan of this. This will cause existing DTs, which are
> > provided by the firmware in an ideal world and may not change at the
> > same rate as the kernel, to generate a warning with new kernels. Not
> > really helpful from the user experience point of view.
>
> I agree. I don't think this warning is useful.
Few users watch the dmesg log, But I totally see your point.
The problem I'm trying to address here is this: when I want to
reset the fec phy, I go look at existing devicetrees. Nearly
all of them use phy-reset-gpios, so that's what I'll use. But,
when I try to upstream the patch, the maintainer will tell me:
"no, that is deprecated, use this other method".
Is there a good solution you can think of which would point
the unwary developer to the correct phy reset method?
See my previous thread here:
https://lkml.org/lkml/2019/7/15/1764
^ permalink raw reply
* Re: [PATCH iproute2-next] tunnel: factorize printout of GRE key and flags
From: David Ahern @ 2019-07-18 17:19 UTC (permalink / raw)
To: Andrea Claudi, netdev; +Cc: stephen, dsahern
In-Reply-To: <547473b8fbab9114f78c1cfe405ccb5f50b0a66b.1562950715.git.aclaudi@redhat.com>
On 7/12/19 11:02 AM, Andrea Claudi wrote:
> print_tunnel() functions in ip6tunnel.c and iptunnel.c contains
> the same code to print out GRE key and flags
>
> This commit factorize the code in a helper function in tunnel.c
>
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
> ip/ip6tunnel.c | 22 ++--------------------
> ip/iptunnel.c | 19 ++-----------------
> ip/tunnel.c | 26 ++++++++++++++++++++++++++
> ip/tunnel.h | 3 +++
> 4 files changed, 33 insertions(+), 37 deletions(-)
applied to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH v3 net-next 13/19] ionic: Add initial ethtool support
From: Shannon Nelson @ 2019-07-18 17:14 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20190718033109.GI6962@lunn.ch>
On 7/17/19 8:31 PM, Andrew Lunn wrote:
> On Fri, Jul 12, 2019 at 10:32:38PM -0700, Shannon Nelson wrote:
>> On 7/8/19 7:30 PM, Andrew Lunn wrote:
>>>> +static int ionic_nway_reset(struct net_device *netdev)
>>>> +{
>>>> + struct lif *lif = netdev_priv(netdev);
>>>> + int err = 0;
>>>> +
>>>> + if (netif_running(netdev))
>>>> + err = ionic_reset_queues(lif);
>>> What does ionic_reset_queues() do? It sounds nothing like restarting
>>> auto negotiation?
>>>
>>> Andrew
>> Basically, it's a rip-it-all-down-and-start-over way of restarting the
>> connection, and is also useful for fixing queues that are misbehaving. It's
>> a little old-fashioned, taken from the ixgbe example, but is effective when
>> there isn't an actual "restart auto-negotiation" command in the firmware.
> O.K. More comments please.
After a little more discussion with the nic connection folks, they've
said to just flap the link to force a new auto-negotiation, so that's
what I'm adding - set state to off, then back on. And yes, another
comment here.
>
> Did you consider throwing the firmware away and just letting Linux
> control the hardware? It would make this all much more transparent and
> debuggable.
>
Yes, the tension between direct driver bit twiddling and smartnic
offloading continues.
sln
^ permalink raw reply
* Re: [PATCH net-next 00/12] mlx5 TLS TX HW offload support
From: Jakub Kicinski @ 2019-07-18 17:08 UTC (permalink / raw)
To: Tariq Toukan
Cc: David Miller, netdev@vger.kernel.org, Eran Ben Elisha,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <1b27ca27-fd33-2e2c-a4c0-ba8878a940db@mellanox.com>
On Thu, 18 Jul 2019 07:40:22 +0000, Tariq Toukan wrote:
> On 7/17/2019 8:41 PM, Jakub Kicinski wrote:
> > On Sun, 7 Jul 2019 06:44:27 +0000, Tariq Toukan wrote:
> >> On 7/6/2019 2:29 AM, David Miller wrote:
> >>> From: Tariq Toukan <tariqt@mellanox.com>
> >>> Date: Fri, 5 Jul 2019 18:30:10 +0300
> >>>
> >>>> This series from Eran and me, adds TLS TX HW offload support to
> >>>> the mlx5 driver.
> >>>
> >>> Series applied, please deal with any further feedback you get from
> >>> Jakub et al.
> >>
> >> I will followup with patches addressing Jakub's feedback.
> >
> > Ping.
> >
>
> Hi Jakub,
>
> I'm waiting for the window to open:
> http://vger.kernel.org/~davem/net-next.html
>
> Do you think these can already go to net as fixes?
Yes, certainly. It's documentation and renaming a stat before it makes
it into an official release.
^ permalink raw reply
* Re: [PATCH v3 net-next 13/19] ionic: Add initial ethtool support
From: Shannon Nelson @ 2019-07-18 17:05 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20190718032137.GG6962@lunn.ch>
On 7/17/19 8:21 PM, Andrew Lunn wrote:
> On Tue, Jul 09, 2019 at 03:42:39PM -0700, Shannon Nelson wrote:
>> On 7/8/19 7:27 PM, Andrew Lunn wrote:
>>>> +static int ionic_get_module_eeprom(struct net_device *netdev,
>>>> + struct ethtool_eeprom *ee,
>>>> + u8 *data)
>>>> +{
>>>> + struct lif *lif = netdev_priv(netdev);
>>>> + struct ionic_dev *idev = &lif->ionic->idev;
>>>> + struct xcvr_status *xcvr;
>>>> + u32 len;
>>>> +
>>>> + /* copy the module bytes into data */
>>>> + xcvr = &idev->port_info->status.xcvr;
>>>> + len = min_t(u32, sizeof(xcvr->sprom), ee->len);
>>>> + memcpy(data, xcvr->sprom, len);
>>> Hi Shannon
>>>
>>> This also looks odd. Where is the call into the firmware to get the
>>> eeprom contents? Even though it is called 'eeprom', the data is not
>>> static. It contains real time diagnostic values, temperature, transmit
>>> power, receiver power, voltages etc.
>> idev->port_info is a memory mapped space that the device keeps up-to-date.
> Hi Shannon
>
> It at least needs a comment. How frequently does the device update
> this chunk of memory? It would be good to comment about that as
> well. Or do MMIO reads block while i2c operations occur to update the
> memory?
The device keeps this updated when changes happen internally so that
there is no need to block on MMIO read. Sure, I'll add a little more
commentary here and perhaps in a couple other similar places.
>
>>>> +
>>>> + dev_dbg(&lif->netdev->dev, "notifyblock eid=0x%llx link_status=0x%x link_speed=0x%x link_down_cnt=0x%x\n",
>>>> + lif->info->status.eid,
>>>> + lif->info->status.link_status,
>>>> + lif->info->status.link_speed,
>>>> + lif->info->status.link_down_count);
>>>> + dev_dbg(&lif->netdev->dev, " port_status id=0x%x status=0x%x speed=0x%x\n",
>>>> + idev->port_info->status.id,
>>>> + idev->port_info->status.status,
>>>> + idev->port_info->status.speed);
>>>> + dev_dbg(&lif->netdev->dev, " xcvr status state=0x%x phy=0x%x pid=0x%x\n",
>>>> + xcvr->state, xcvr->phy, xcvr->pid);
>>>> + dev_dbg(&lif->netdev->dev, " port_config state=0x%x speed=0x%x mtu=0x%x an_enable=0x%x fec_type=0x%x pause_type=0x%x loopback_mode=0x%x\n",
>>>> + idev->port_info->config.state,
>>>> + idev->port_info->config.speed,
>>>> + idev->port_info->config.mtu,
>>>> + idev->port_info->config.an_enable,
>>>> + idev->port_info->config.fec_type,
>>>> + idev->port_info->config.pause_type,
>>>> + idev->port_info->config.loopback_mode);
>>> It is a funny place to have all the debug code.
>> Someone wanted a hook for getting some link info on the fly on request.
> I would suggest deleting it all. Most of it is already available via
> ethtool.
>
Sure.
sln
^ permalink raw reply
* Re: [PATCH net-next iproute2 v2 0/3] net/sched: Introduce tc connection tracking
From: David Ahern @ 2019-07-18 16:58 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, Paul Blakey
Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
netdev@vger.kernel.org, David Miller, Aaron Conole, Zhike Wang,
Rony Efraim, nst-kernel@redhat.com, John Hurley, Simon Horman,
Justin Pettit
In-Reply-To: <20190718165233.GT3390@localhost.localdomain>
On 7/18/19 10:52 AM, Marcelo Ricardo Leitner wrote:
> On Thu, Jul 18, 2019 at 03:00:34PM +0000, Paul Blakey wrote:
>> Hey guys,
>>
>> any more comments?
>
> From my side, just the man page on tc-ct(8) that is missing.
> Everything else seems to be in place.
>
> Thanks,
> Marcelo
>
Paul: If there are no other comments, I'll push these to iproute2-next
later today. In that case, send the man page as a follow on patch.
^ permalink raw reply
* Re: [PATCH net-next iproute2 v2 0/3] net/sched: Introduce tc connection tracking
From: Marcelo Ricardo Leitner @ 2019-07-18 16:52 UTC (permalink / raw)
To: Paul Blakey
Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
netdev@vger.kernel.org, David Miller, Aaron Conole, Zhike Wang,
Rony Efraim, nst-kernel@redhat.com, John Hurley, Simon Horman,
Justin Pettit
In-Reply-To: <9286cbad-6821-786a-6882-d2bf56b3cf05@mellanox.com>
On Thu, Jul 18, 2019 at 03:00:34PM +0000, Paul Blakey wrote:
> Hey guys,
>
> any more comments?
From my side, just the man page on tc-ct(8) that is missing.
Everything else seems to be in place.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH] net: fec: generate warning when using deprecated phy reset
From: Fabio Estevam @ 2019-07-18 16:52 UTC (permalink / raw)
To: Lucas Stach
Cc: Sven Van Asbroeck, Fugang Duan, David S . Miller, netdev,
linux-kernel
In-Reply-To: <1563468471.2676.36.camel@pengutronix.de>
On Thu, Jul 18, 2019 at 1:49 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Donnerstag, den 18.07.2019, 10:34 -0400 schrieb Sven Van Asbroeck:
> > Allowing the fec to reset its PHY via the phy-reset-gpios
> > devicetree property is deprecated. To improve developer
> > awareness, generate a warning whenever the deprecated
> > property is used.
>
> Not really a fan of this. This will cause existing DTs, which are
> provided by the firmware in an ideal world and may not change at the
> same rate as the kernel, to generate a warning with new kernels. Not
> really helpful from the user experience point of view.
I agree. I don't think this warning is useful.
^ permalink raw reply
* Re: KASAN: use-after-free Read in nr_insert_socket
From: Cong Wang @ 2019-07-18 16:48 UTC (permalink / raw)
To: syzbot
Cc: David Miller, linux-hams, LKML, Linux Kernel Network Developers,
Ralf Baechle, syzkaller-bugs
In-Reply-To: <00000000000035f65d058df39aed@google.com>
On Thu, Jul 18, 2019 at 5:18 AM syzbot
<syzbot+9399c158fcc09b21d0d2@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: a5b64700 fix: taprio: Change type of txtime-delay paramete..
> git tree: net
> console output: https://syzkaller.appspot.com/x/log.txt?x=1588b458600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=87305c3ca9c25c70
> dashboard link: https://syzkaller.appspot.com/bug?extid=9399c158fcc09b21d0d2
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=105a61a4600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=153ef948600000
>
> The bug was bisected to:
>
> commit c8c8218ec5af5d2598381883acbefbf604e56b5e
> Author: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Thu Jun 27 21:30:58 2019 +0000
>
> netrom: fix a memory leak in nr_rx_frame()
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=159ef948600000
> final crash: https://syzkaller.appspot.com/x/report.txt?x=179ef948600000
> console output: https://syzkaller.appspot.com/x/log.txt?x=139ef948600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+9399c158fcc09b21d0d2@syzkaller.appspotmail.com
> Fixes: c8c8218ec5af ("netrom: fix a memory leak in nr_rx_frame()")
>
> ==================================================================
> BUG: KASAN: use-after-free in atomic_read
> /./include/asm-generic/atomic-instrumented.h:26 [inline]
> BUG: KASAN: use-after-free in refcount_inc_not_zero_checked+0x81/0x200
> /lib/refcount.c:123
> Read of size 4 at addr ffff8880a5d3f380 by task swapper/1/0
>
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.2.0+ #89
> 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
> print_address_description.cold+0xd4/0x306 /mm/kasan/report.c:351
> __kasan_report.cold+0x1b/0x36 /mm/kasan/report.c:482
> kasan_report+0x12/0x20 /mm/kasan/common.c:612
> check_memory_region_inline /mm/kasan/generic.c:185 [inline]
> check_memory_region+0x134/0x1a0 /mm/kasan/generic.c:192
> __kasan_check_read+0x11/0x20 /mm/kasan/common.c:92
> atomic_read /./include/asm-generic/atomic-instrumented.h:26 [inline]
> refcount_inc_not_zero_checked+0x81/0x200 /lib/refcount.c:123
> refcount_inc_checked+0x17/0x70 /lib/refcount.c:156
> sock_hold /./include/net/sock.h:649 [inline]
> sk_add_node /./include/net/sock.h:701 [inline]
> nr_insert_socket+0x2d/0xe0 /net/netrom/af_netrom.c:137
Looks like nr_insert_socket() doesn't hold a refcnt before inserting
it into a global list.
Let me think about how to fix this.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: fec: generate warning when using deprecated phy reset
From: Lucas Stach @ 2019-07-18 16:47 UTC (permalink / raw)
To: Sven Van Asbroeck, Fugang Duan; +Cc: David S . Miller, netdev, linux-kernel
In-Reply-To: <20190718143428.2392-1-TheSven73@gmail.com>
Am Donnerstag, den 18.07.2019, 10:34 -0400 schrieb Sven Van Asbroeck:
> Allowing the fec to reset its PHY via the phy-reset-gpios
> devicetree property is deprecated. To improve developer
> awareness, generate a warning whenever the deprecated
> property is used.
Not really a fan of this. This will cause existing DTs, which are
provided by the firmware in an ideal world and may not change at the
same rate as the kernel, to generate a warning with new kernels. Not
really helpful from the user experience point of view.
Regards,
Lucas
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 38f10f7dcbc3..00e1b5e4ef71 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3244,6 +3244,12 @@ static int fec_reset_phy(struct platform_device *pdev)
> > else if (!gpio_is_valid(phy_reset))
> > return 0;
>
> > + /* Recommended way to provide a PHY reset:
> > + * - create a phy devicetree node, and link it to its fec (phy-handle)
> > + * - add your reset gpio to the phy devicetree node
> > + */
> > + dev_warn(&pdev->dev, "devicetree: phy-reset-gpios is deprecated\n");
> +
> > err = of_property_read_u32(np, "phy-reset-post-delay", &phy_post_delay);
> > /* valid reset duration should be less than 1s */
> > if (!err && phy_post_delay > 1000)
^ permalink raw reply
* Re: [PATCH] net: dsa: sja1105: Fix missing unlock on error in sk_buff()
From: Vivien Didelot @ 2019-07-18 16:42 UTC (permalink / raw)
To: Wei Yongjun
Cc: Andrew Lunn, Florian Fainelli, Vladimir Oltean, Wei Yongjun,
netdev, kernel-janitors
In-Reply-To: <20190717062956.127446-1-weiyongjun1@huawei.com>
On Wed, 17 Jul 2019 06:29:56 +0000, Wei Yongjun <weiyongjun1@huawei.com> wrote:
> Add the missing unlock before return from function sk_buff()
> in the error handling case.
>
> Fixes: f3097be21bf1 ("net: dsa: sja1105: Add a state machine for RX timestamping")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [RFC PATCH 4/5] PTP: Add flag for non-periodic output
From: Richard Cochran @ 2019-07-18 16:41 UTC (permalink / raw)
To: Felipe Balbi
Cc: netdev, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, x86, linux-kernel, Christopher S . Hall
In-Reply-To: <87ftn3iuqp.fsf@linux.intel.com>
On Thu, Jul 18, 2019 at 11:59:10AM +0300, Felipe Balbi wrote:
> no problem, anything in particular in mind? Just create new versions of
> all the IOCTLs so we can actually use the reserved fields in the future?
Yes, please!
Thanks,
Richard
^ permalink raw reply
* [PATCH] openvswitch: Fix a possible memory leak on dst_cache
From: Haishuang Yan @ 2019-07-18 16:07 UTC (permalink / raw)
To: Pravin B Shelar, David S. Miller; +Cc: netdev, linux-kernel, Haishuang Yan
dst_cache should be destroyed when fail to add flow actions.
Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel")
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
net/openvswitch/flow_netlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d7559c6..1fd1cdd 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2608,6 +2608,7 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
sizeof(*ovs_tun), log);
if (IS_ERR(a)) {
dst_release((struct dst_entry *)tun_dst);
+ dst_cache_destroy(&tun_dst->u.tun_info.dst_cache);
return PTR_ERR(a);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] MAINTAINERS: update netsec driver
From: Ard Biesheuvel @ 2019-07-18 16:06 UTC (permalink / raw)
To: Jassi Brar
Cc: Ilias Apalodimas, <netdev@vger.kernel.org>, David S. Miller
In-Reply-To: <CAJe_ZhdFBUWQwf+OcDX_0_wYTpTqHJvqJi2QE3CP+8rwXCLjMw@mail.gmail.com>
On Thu, 18 Jul 2019 at 16:52, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>
> On Thu, 18 Jul 2019 at 09:38, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Add myself to maintainers since i provided the XDP and page_pool
> > implementation
> >
> Yes, please.
>
> Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
^ permalink raw reply
* Re: [PATCH bpf v2] bpf: fix narrower loads on s390
From: Y Song @ 2019-07-18 15:24 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev, gor, heiko.carstens
In-Reply-To: <20190718150103.84837-1-iii@linux.ibm.com>
On Thu, Jul 18, 2019 at 8:01 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> The very first check in test_pkt_md_access is failing on s390, which
> happens because loading a part of a struct __sk_buff field produces
> an incorrect result.
>
> The preprocessed code of the check is:
>
> {
> __u8 tmp = *((volatile __u8 *)&skb->len +
> ((sizeof(skb->len) - sizeof(__u8)) / sizeof(__u8)));
> if (tmp != ((*(volatile __u32 *)&skb->len) & 0xFF)) return 2;
> };
>
> clang generates the following code for it:
>
> 0: 71 21 00 03 00 00 00 00 r2 = *(u8 *)(r1 + 3)
> 1: 61 31 00 00 00 00 00 00 r3 = *(u32 *)(r1 + 0)
> 2: 57 30 00 00 00 00 00 ff r3 &= 255
> 3: 5d 23 00 1d 00 00 00 00 if r2 != r3 goto +29 <LBB0_10>
>
> Finally, verifier transforms it to:
>
> 0: (61) r2 = *(u32 *)(r1 +104)
> 1: (bc) w2 = w2
> 2: (74) w2 >>= 24
> 3: (bc) w2 = w2
> 4: (54) w2 &= 255
> 5: (bc) w2 = w2
>
> The problem is that when verifier emits the code to replace a partial
> load of a struct __sk_buff field (*(u8 *)(r1 + 3)) with a full load of
> struct sk_buff field (*(u32 *)(r1 + 104)), an optional shift and a
> bitwise AND, it assumes that the machine is little endian and
> incorrectly decides to use a shift.
>
> Adjust shift count calculation to account for endianness.
>
> Fixes: 31fd85816dbe ("bpf: permits narrower load from bpf program context fields")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/filter.h | 13 +++++++++++++
> kernel/bpf/verifier.c | 4 ++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index ff65d22cf336..4fe88e43f0fe 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -24,6 +24,8 @@
>
> #include <net/sch_generic.h>
>
> +#include <asm/byteorder.h>
> +
> #include <uapi/linux/filter.h>
> #include <uapi/linux/bpf.h>
>
> @@ -1216,4 +1218,15 @@ struct bpf_sockopt_kern {
> s32 retval;
> };
>
> +static inline u8 bpf_narrower_load_shift(u32 size_default, u32 size, u32 off)
> +{
> + u8 load_off = off & (size_default - 1);
> +
> +#ifdef __LITTLE_ENDIAN
> + return load_off * 8;
> +#else
> + return (size_default - (load_off + size)) * 8;
> +#endif
> +}
> +
> #endif /* __LINUX_FILTER_H__ */
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5900cbb966b1..48edc9c9a879 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8616,8 +8616,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> }
>
> if (is_narrower_load && size < target_size) {
> - u8 shift = (off & (size_default - 1)) * 8;
> -
> + u8 shift = bpf_narrower_load_shift(size_default, size,
> + off);
> if (ctx_field_size <= 4) {
> if (shift)
> insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH v6 0/5] net: macb: cover letter
From: Andrew Lunn @ 2019-07-18 15:13 UTC (permalink / raw)
To: Parshuram Thombare
Cc: nicolas.ferre, davem, f.fainelli, linux, netdev, hkallweit1,
linux-kernel, rafalc, piotrs, aniljoy, arthurm, stevenh, mparab
In-Reply-To: <1562769391-31803-1-git-send-email-pthombar@cadence.com>
On Wed, Jul 10, 2019 at 03:36:31PM +0100, Parshuram Thombare wrote:
> Hello !
>
> This is 6th version of patch set containing following patches
> for Cadence ethernet controller driver.
Hi Parshuram
One thing which was never clear is how you are testing the features
you are adding. Please could you describe your test setup and how each
new feature is tested using that hardware. I'm particularly interested
in what C45 device are you using? But i expect Russell would like to
know more about SFP modules you are using. Do you have any which
require 1000BaseX, 2500BaseX, or provide copper 1G?
Thanks
Andrew
^ permalink raw reply
* Re: regression with napi/softirq ?
From: Eric Dumazet @ 2019-07-18 15:08 UTC (permalink / raw)
To: Sudip Mukherjee, Eric Dumazet
Cc: Thomas Gleixner, Peter Zijlstra (Intel), David S. Miller, netdev,
linux-kernel
In-Reply-To: <CADVatmPQRf9A9z1LbHe5cd+bFLrPGG12YxPh2-yXAj_C9s8ZeA@mail.gmail.com>
On 7/18/19 2:55 PM, Sudip Mukherjee wrote:
> Thanks Eric. But there is no improvement in delay between
> softirq_raise and softirq_entry with this change.
> But moving to a later kernel (linus master branch? ) like Thomas has
> said in the other mail might be difficult atm. I can definitely
> move to v4.14.133 if that helps. Thomas ?
If you are tracking max latency then I guess you have to tweak SOFTIRQ_NOW_MASK
to include NET_RX_SOFTIRQ
The patch I gave earlier would only lower the probability of events, not completely get rid of them.
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 0427a86743a46b7e1891f7b6c1ff585a8a1695f5..302046dd8d7e6740e466c422954f22565fe19e69 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -81,7 +81,7 @@ static void wakeup_softirqd(void)
* right now. Let ksoftirqd handle this at its own rate, to get fairness,
* unless we're doing some of the synchronous softirqs.
*/
-#define SOFTIRQ_NOW_MASK ((1 << HI_SOFTIRQ) | (1 << TASKLET_SOFTIRQ))
+#define SOFTIRQ_NOW_MASK ((1 << HI_SOFTIRQ) | (1 << TASKLET_SOFTIRQ) | (1 << NET_RX_SOFTIRQ))
static bool ksoftirqd_running(unsigned long pending)
{
struct task_struct *tsk = __this_cpu_read(ksoftirqd);
^ permalink raw reply related
* [PATCH bpf v2] bpf: fix narrower loads on s390
From: Ilya Leoshkevich @ 2019-07-18 15:01 UTC (permalink / raw)
To: bpf, netdev, ys114321; +Cc: gor, heiko.carstens, Ilya Leoshkevich
The very first check in test_pkt_md_access is failing on s390, which
happens because loading a part of a struct __sk_buff field produces
an incorrect result.
The preprocessed code of the check is:
{
__u8 tmp = *((volatile __u8 *)&skb->len +
((sizeof(skb->len) - sizeof(__u8)) / sizeof(__u8)));
if (tmp != ((*(volatile __u32 *)&skb->len) & 0xFF)) return 2;
};
clang generates the following code for it:
0: 71 21 00 03 00 00 00 00 r2 = *(u8 *)(r1 + 3)
1: 61 31 00 00 00 00 00 00 r3 = *(u32 *)(r1 + 0)
2: 57 30 00 00 00 00 00 ff r3 &= 255
3: 5d 23 00 1d 00 00 00 00 if r2 != r3 goto +29 <LBB0_10>
Finally, verifier transforms it to:
0: (61) r2 = *(u32 *)(r1 +104)
1: (bc) w2 = w2
2: (74) w2 >>= 24
3: (bc) w2 = w2
4: (54) w2 &= 255
5: (bc) w2 = w2
The problem is that when verifier emits the code to replace a partial
load of a struct __sk_buff field (*(u8 *)(r1 + 3)) with a full load of
struct sk_buff field (*(u32 *)(r1 + 104)), an optional shift and a
bitwise AND, it assumes that the machine is little endian and
incorrectly decides to use a shift.
Adjust shift count calculation to account for endianness.
Fixes: 31fd85816dbe ("bpf: permits narrower load from bpf program context fields")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
include/linux/filter.h | 13 +++++++++++++
kernel/bpf/verifier.c | 4 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ff65d22cf336..4fe88e43f0fe 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -24,6 +24,8 @@
#include <net/sch_generic.h>
+#include <asm/byteorder.h>
+
#include <uapi/linux/filter.h>
#include <uapi/linux/bpf.h>
@@ -1216,4 +1218,15 @@ struct bpf_sockopt_kern {
s32 retval;
};
+static inline u8 bpf_narrower_load_shift(u32 size_default, u32 size, u32 off)
+{
+ u8 load_off = off & (size_default - 1);
+
+#ifdef __LITTLE_ENDIAN
+ return load_off * 8;
+#else
+ return (size_default - (load_off + size)) * 8;
+#endif
+}
+
#endif /* __LINUX_FILTER_H__ */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5900cbb966b1..48edc9c9a879 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8616,8 +8616,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
}
if (is_narrower_load && size < target_size) {
- u8 shift = (off & (size_default - 1)) * 8;
-
+ u8 shift = bpf_narrower_load_shift(size_default, size,
+ off);
if (ctx_field_size <= 4) {
if (shift)
insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH net-next iproute2 v2 0/3] net/sched: Introduce tc connection tracking
From: Paul Blakey @ 2019-07-18 15:00 UTC (permalink / raw)
To: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev@vger.kernel.org, David Miller,
Aaron Conole, Zhike Wang
Cc: Rony Efraim, nst-kernel@redhat.com, John Hurley, Simon Horman,
Justin Pettit
In-Reply-To: <1562832867-32347-1-git-send-email-paulb@mellanox.com>
Hey guys,
any more comments?
thanks,
Paul.
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: update netsec driver
From: Jassi Brar @ 2019-07-18 14:52 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: <netdev@vger.kernel.org>, David S. Miller, Ard Biesheuvel
In-Reply-To: <1563460710-28454-1-git-send-email-ilias.apalodimas@linaro.org>
On Thu, 18 Jul 2019 at 09:38, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Add myself to maintainers since i provided the XDP and page_pool
> implementation
>
Yes, please.
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply
* Re: [PATCH] virtio-net: parameterize min ring num_free for virtio receive
From: Michael S. Tsirkin @ 2019-07-18 14:43 UTC (permalink / raw)
To: Jason Wang
Cc: ? jiang, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net, jakub.kicinski@netronome.com,
hawk@kernel.org, john.fastabend@gmail.com, kafai@fb.com,
songliubraving@fb.com, yhs@fb.com,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, xdp-newbies@vger.kernel.org,
bpf@vger.kernel.org, jiangran.jr@alibaba-inc.com
In-Reply-To: <20190718103641-mutt-send-email-mst@kernel.org>
On Thu, Jul 18, 2019 at 10:42:47AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jul 18, 2019 at 10:01:05PM +0800, Jason Wang wrote:
> >
> > On 2019/7/18 下午9:04, Michael S. Tsirkin wrote:
> > > On Thu, Jul 18, 2019 at 12:55:50PM +0000, ? jiang wrote:
> > > > This change makes ring buffer reclaim threshold num_free configurable
> > > > for better performance, while it's hard coded as 1/2 * queue now.
> > > > According to our test with qemu + dpdk, packet dropping happens when
> > > > the guest is not able to provide free buffer in avail ring timely.
> > > > Smaller value of num_free does decrease the number of packet dropping
> > > > during our test as it makes virtio_net reclaim buffer earlier.
> > > >
> > > > At least, we should leave the value changeable to user while the
> > > > default value as 1/2 * queue is kept.
> > > >
> > > > Signed-off-by: jiangkidd<jiangkidd@hotmail.com>
> > > That would be one reason, but I suspect it's not the
> > > true one. If you need more buffer due to jitter
> > > then just increase the queue size. Would be cleaner.
> > >
> > >
> > > However are you sure this is the reason for
> > > packet drops? Do you see them dropped by dpdk
> > > due to lack of space in the ring? As opposed to
> > > by guest?
> > >
> > >
> >
> > Besides those, this patch depends on the user to choose a suitable threshold
> > which is not good. You need either a good value with demonstrated numbers or
> > something smarter.
> >
> > Thanks
>
> I do however think that we have a problem right now: try_fill_recv can
> take up a long time during which net stack does not run at all. Imagine
> a 1K queue - we are talking 512 packets. That's exceessive. napi poll
> weight solves a similar problem, so it might make sense to cap this at
> napi_poll_weight.
>
> Which will allow tweaking it through a module parameter as a
> side effect :) Maybe just do NAPI_POLL_WEIGHT.
Or maybe NAPI_POLL_WEIGHT/2 like we do at half the queue ;). Please
experiment, measure performance and let the list know
> Need to be careful though: queues can also be small and I don't think we
> want to exceed queue size / 2, or maybe queue size - napi_poll_weight.
> Definitely must not exceed the full queue size.
>
> --
> MST
^ permalink raw reply
* Re: [PATCH] virtio-net: parameterize min ring num_free for virtio receive
From: Michael S. Tsirkin @ 2019-07-18 14:42 UTC (permalink / raw)
To: Jason Wang
Cc: ? jiang, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net, jakub.kicinski@netronome.com,
hawk@kernel.org, john.fastabend@gmail.com, kafai@fb.com,
songliubraving@fb.com, yhs@fb.com,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, xdp-newbies@vger.kernel.org,
bpf@vger.kernel.org, jiangran.jr@alibaba-inc.com
In-Reply-To: <bdd30ef5-4f69-8218-eed0-38c6daac42db@redhat.com>
On Thu, Jul 18, 2019 at 10:01:05PM +0800, Jason Wang wrote:
>
> On 2019/7/18 下午9:04, Michael S. Tsirkin wrote:
> > On Thu, Jul 18, 2019 at 12:55:50PM +0000, ? jiang wrote:
> > > This change makes ring buffer reclaim threshold num_free configurable
> > > for better performance, while it's hard coded as 1/2 * queue now.
> > > According to our test with qemu + dpdk, packet dropping happens when
> > > the guest is not able to provide free buffer in avail ring timely.
> > > Smaller value of num_free does decrease the number of packet dropping
> > > during our test as it makes virtio_net reclaim buffer earlier.
> > >
> > > At least, we should leave the value changeable to user while the
> > > default value as 1/2 * queue is kept.
> > >
> > > Signed-off-by: jiangkidd<jiangkidd@hotmail.com>
> > That would be one reason, but I suspect it's not the
> > true one. If you need more buffer due to jitter
> > then just increase the queue size. Would be cleaner.
> >
> >
> > However are you sure this is the reason for
> > packet drops? Do you see them dropped by dpdk
> > due to lack of space in the ring? As opposed to
> > by guest?
> >
> >
>
> Besides those, this patch depends on the user to choose a suitable threshold
> which is not good. You need either a good value with demonstrated numbers or
> something smarter.
>
> Thanks
I do however think that we have a problem right now: try_fill_recv can
take up a long time during which net stack does not run at all. Imagine
a 1K queue - we are talking 512 packets. That's exceessive. napi poll
weight solves a similar problem, so it might make sense to cap this at
napi_poll_weight.
Which will allow tweaking it through a module parameter as a
side effect :) Maybe just do NAPI_POLL_WEIGHT.
Need to be careful though: queues can also be small and I don't think we
want to exceed queue size / 2, or maybe queue size - napi_poll_weight.
Definitely must not exceed the full queue size.
--
MST
^ permalink raw reply
* [PATCH] MAINTAINERS: update netsec driver
From: Ilias Apalodimas @ 2019-07-18 14:38 UTC (permalink / raw)
To: netdev, jaswinder.singh, davem; +Cc: ard.biesheuvel, Ilias Apalodimas
Add myself to maintainers since i provided the XDP and page_pool
implementation
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 211ea3a199bd..64f659d8346c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14789,6 +14789,7 @@ F: Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
SOCIONEXT (SNI) NETSEC NETWORK DRIVER
M: Jassi Brar <jaswinder.singh@linaro.org>
+M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
L: netdev@vger.kernel.org
S: Maintained
F: drivers/net/ethernet/socionext/netsec.c
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox