* Re: [PATCH net-next 00/19] Add support for Aquantia AQtion USB to 5/2.5GbE devices
From: Igor Russkikh @ 2018-10-08 7:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Simon Edelhaus, Nadezhda Krupnina,
Dmitry.Bezrukov
In-Reply-To: <20181006175122.GH6990@lunn.ch>
Hi Andrew,
> Nice patch set, well broken up, easy to review.
>
Thanks a lot for your detailed review, your comments are really useful!
I'll respond to some of your comments separately.
Regards,
Igor
^ permalink raw reply
* [v3, 3/6] net: dpaa2: fix dependency of config FSL_DPAA2_ETH
From: Yangbo Lu @ 2018-10-08 7:44 UTC (permalink / raw)
To: linux-kernel, devel, netdev, Richard Cochran, David S . Miller,
Ioana Radulescu, Greg Kroah-Hartman, Andrew Lunn
Cc: Yangbo Lu
In-Reply-To: <20181008074430.34379-1-yangbo.lu@nxp.com>
The NETDEVICES dependency and ETHERNET dependency hadn't
been required since dpaa2-eth was moved out of staging.
Also allowed COMPILE_TEST for dpaa2-eth.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
drivers/net/ethernet/freescale/dpaa2/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig b/drivers/net/ethernet/freescale/dpaa2/Kconfig
index 67e6461..a7f365d 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Kconfig
+++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig
@@ -1,7 +1,7 @@
config FSL_DPAA2_ETH
tristate "Freescale DPAA2 Ethernet"
depends on FSL_MC_BUS && FSL_MC_DPIO
- depends on NETDEVICES && ETHERNET
+ depends on ARCH_LAYERSCAPE || COMPILE_TEST
help
This is the DPAA2 Ethernet driver supporting Freescale SoCs
with DPAA2 (DataPath Acceleration Architecture v2).
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] inet: do not set backlog if listen fails
From: Yafang Shao @ 2018-10-08 14:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David Miller, netdev, LKML
In-Reply-To: <CANn89iLUKmiP==WP=cf9KYwMaNDbsU2M=b07dPU+DspfBGSsAg@mail.gmail.com>
On Mon, Oct 8, 2018 at 10:48 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Oct 8, 2018 at 3:47 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> >
> > Pls. correct me if I missed something.
> >
>
> You missed that this variable must be set before listener is setup.
> Same feedback than prior version really (when you trived to move
> around sk_ack_backlog clearing)
>
> You are changing inet_csk_listen_start() but this is wrong...
Got it.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next] inet: do not set backlog if listen fails
From: Eric Dumazet @ 2018-10-08 14:47 UTC (permalink / raw)
To: Yafang Shao; +Cc: Eric Dumazet, David Miller, netdev, LKML
In-Reply-To: <CALOAHbAAwr1rsPhHT4R7wn3SYJ220C-i92qObSA3Cvp99M8i8w@mail.gmail.com>
On Mon, Oct 8, 2018 at 3:47 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
>
> Pls. correct me if I missed something.
>
You missed that this variable must be set before listener is setup.
Same feedback than prior version really (when you trived to move
around sk_ack_backlog clearing)
You are changing inet_csk_listen_start() but this is wrong...
^ permalink raw reply
* [PATCH] rtl8xxxu: Remove set but not used variables 'usedesc40' and 'seq_number'
From: YueHaibing @ 2018-10-08 14:16 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo
Cc: YueHaibing, linux-wireless, netdev, linux-kernel, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function 'rtl8xxxu_tx':
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4925:7: warning:
variable 'usedesc40' set but not used [-Wunused-but-set-variable]
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4921:6: warning:
variable 'seq_number' set but not used [-Wunused-but-set-variable]
'usedesc40' and 'seq_number' are not used any more after
commit b59415c2dd08 ("rtl8xxxu: Split filling of TX descriptors into separate functions")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 73f6fc0..56040b1 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4918,11 +4918,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
struct device *dev = &priv->udev->dev;
u32 queue, rts_rate;
u16 pktlen = skb->len;
- u16 seq_number;
u16 rate_flag = tx_info->control.rates[0].flags;
int tx_desc_size = priv->fops->tx_desc_size;
int ret;
- bool usedesc40, ampdu_enable, sgi = false, short_preamble = false;
+ bool ampdu_enable, sgi = false, short_preamble = false;
if (skb_headroom(skb) < tx_desc_size) {
dev_warn(dev,
@@ -4946,7 +4945,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
if (ieee80211_is_action(hdr->frame_control))
rtl8xxxu_dump_action(dev, hdr);
- usedesc40 = (tx_desc_size == 40);
tx_info->rate_driver_data[0] = hw;
if (control && control->sta)
@@ -5013,7 +5011,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
else
rts_rate = 0;
- seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
ampdu_enable, rts_rate);
^ permalink raw reply related
* Re: [PATCH net-next v3] wireless-drivers: rtnetlink wifi simulation device
From: Sergey Matyukevich @ 2018-10-08 13:56 UTC (permalink / raw)
To: Joel Fernandes
Cc: Cody Schuffelen, Johannes Berg, Kalle Valo, David S . Miller,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, kernel-team@android.com
In-Reply-To: <CAJWu+ooTRUxj2PAiUkgjTLb+VC9OoBpkj9-_ZoC-oCS3owc5Hg@mail.gmail.com>
> > Hi Cody,
> >
> >> drivers/net/wireless/Kconfig | 7 +
> >> drivers/net/wireless/Makefile | 2 +
> >> drivers/net/wireless/virt_wifi.c | 618 +++++++++++++++++++++++++++++++
> >> 3 files changed, 627 insertions(+)
> >> create mode 100644 drivers/net/wireless/virt_wifi.c
> >
> > I did a quick check of your patch using checkpatch kernel tool,
> > here is a summary of its output:
> >
> > $ ./scripts/checkpatch.pl --strict test.patch
> > ...
> > total: 165 errors, 428 warnings, 9 checks, 634 lines checked
> >
> > Most part of those complaints is about either whitespaces or code
> > idents. I am not sure whether this is a patch itself or email client.
> > So could you please take a look and run checkpatch on your side.
> >
>
> Yeah, it could be his email client, weird though because if I pull the
> patch from the kernel.org archive's mbox though, I don't get any
> errors except the MAINTAINERS file thing:
>
> wget https://lore.kernel.org/lkml/20181004195906.201895-1-schuffelen@google.com/raw
> -O /tmp/tmp.patch
> ./scripts/checkpatch.pl --strict /tmp/tmp.patch
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #167:
> new file mode 100644
>
> total: 0 errors, 1 warnings, 0 checks, 634 lines checked
Hi Cody and Joel,
Please ignore my comment regarding the whitespace issues in the patch.
I don't see those issues when downloading the patch via gmail server.
So the issue was on my side.
Regards,
Sergey
^ permalink raw reply
* Re: [PATCH ipsec-next] Clear secpath on loopback_xmit
From: Steffen Klassert @ 2018-10-08 6:40 UTC (permalink / raw)
To: Benedict Wong; +Cc: netdev, nharold, lorenzo
In-Reply-To: <20181005182327.14819-1-benedictwong@google.com>
On Fri, Oct 05, 2018 at 11:23:28AM -0700, Benedict Wong wrote:
> This patch clears the skb->sp when transmitted over loopback. This
> ensures that the loopback-ed packet does not have any secpath
> information from the outbound transforms.
>
> At present, this causes XFRM tunnel mode packets to be dropped with
> XFRMINNOPOLS, due to the outbound state being in the secpath, without
> a matching inbound policy. Clearing the secpath ensures that all states
> added to the secpath are exclusively from the inbound processing.
This looks like a fix, so the target should be the ipsec tree,
not ipsec-next.
>
> Tests: xfrm tunnel mode tests added for loopback:
> https://android-review.googlesource.com/c/kernel/tests/+/777328
> Signed-off-by: Benedict Wong <benedictwong@google.com>
Please add a 'Fixes:' tag.
> ---
> drivers/net/loopback.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> index 30612497643c..a6bf54df94bd 100644
> --- a/drivers/net/loopback.c
> +++ b/drivers/net/loopback.c
> @@ -50,6 +50,7 @@
> #include <linux/ethtool.h>
> #include <net/sock.h>
> #include <net/checksum.h>
> +#include <net/xfrm.h>
> #include <linux/if_ether.h> /* For the statistics structure. */
> #include <linux/if_arp.h> /* For ARPHRD_ETHER */
> #include <linux/ip.h>
> @@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
> */
> skb_dst_force(skb);
>
> + // Clear secpath to ensure xfrm policy check not tainted by outbound SAs.
Comments should use /* ... */, like it is done below.
> + secpath_reset(skb);
> +
> skb->protocol = eth_type_trans(skb, dev);
>
> /* it's OK to use per_cpu_ptr() because BHs are off */
> --
> 2.19.0.605.g01d371f741-goog
^ permalink raw reply
* [PATCH] net: aquantia: remove some redundant variable initializations
From: Colin King @ 2018-10-08 13:35 UTC (permalink / raw)
To: David S . Miller, Igor Russkikh, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are several variables being initialized that are being set later
and hence the initialization is redundant and can be removed. Remove
then.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 750007513f9d..1d5d6b8df855 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -84,7 +84,7 @@ static int aq_pci_probe_get_hw_by_id(struct pci_dev *pdev,
const struct aq_hw_ops **ops,
const struct aq_hw_caps_s **caps)
{
- int i = 0;
+ int i;
if (pdev->vendor != PCI_VENDOR_ID_AQUANTIA)
return -EINVAL;
@@ -107,7 +107,7 @@ static int aq_pci_probe_get_hw_by_id(struct pci_dev *pdev,
int aq_pci_func_init(struct pci_dev *pdev)
{
- int err = 0;
+ int err;
err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
if (!err) {
@@ -141,7 +141,7 @@ int aq_pci_func_alloc_irq(struct aq_nic_s *self, unsigned int i,
char *name, void *aq_vec, cpumask_t *affinity_mask)
{
struct pci_dev *pdev = self->pdev;
- int err = 0;
+ int err;
if (pdev->msix_enabled || pdev->msi_enabled)
err = request_irq(pci_irq_vector(pdev, i), aq_vec_isr, 0,
@@ -164,7 +164,7 @@ int aq_pci_func_alloc_irq(struct aq_nic_s *self, unsigned int i,
void aq_pci_func_free_irqs(struct aq_nic_s *self)
{
struct pci_dev *pdev = self->pdev;
- unsigned int i = 0U;
+ unsigned int i;
for (i = 32U; i--;) {
if (!((1U << i) & self->msix_entry_mask))
@@ -194,8 +194,8 @@ static void aq_pci_free_irq_vectors(struct aq_nic_s *self)
static int aq_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *pci_id)
{
- struct aq_nic_s *self = NULL;
- int err = 0;
+ struct aq_nic_s *self;
+ int err;
struct net_device *ndev;
resource_size_t mmio_pa;
u32 bar;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next] mt76x0: pci: fix set external PA I/O current
From: Lorenzo Bianconi @ 2018-10-08 13:29 UTC (permalink / raw)
To: YueHaibing
Cc: Kalle Valo, Felix Fietkau, Stanislaw Gruszka,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1539004909-195005-1-git-send-email-yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/mediatek/mt76/mt76x0/pci.c: In function 'mt76x0e_register_device':
> drivers/net/wireless/mediatek/mt76/mt76x0/pci.c:107:8: warning:
> variable 'data' set but not used [-Wunused-but-set-variable]
>
> It seems correct value to write is 'data'
>
> Fixes: 2b2cb40bcd7d ("mt76x0: pci: add hw initialization at bootstrap")
> Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply
* [PATCH net-next] mt76x0: pci: fix set external PA I/O current
From: YueHaibing @ 2018-10-08 13:21 UTC (permalink / raw)
To: Kalle Valo, Felix Fietkau, Lorenzo Bianconi, Stanislaw Gruszka
Cc: YueHaibing, linux-wireless, kernel-janitors, netdev, linux-kernel
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/mediatek/mt76/mt76x0/pci.c: In function 'mt76x0e_register_device':
drivers/net/wireless/mediatek/mt76/mt76x0/pci.c:107:8: warning:
variable 'data' set but not used [-Wunused-but-set-variable]
It seems correct value to write is 'data'
Fixes: 2b2cb40bcd7d ("mt76x0: pci: add hw initialization at bootstrap")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/mediatek/mt76/mt76x0/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
index 87997cd..0426c68 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
@@ -106,12 +106,12 @@ static int mt76x0e_register_device(struct mt76x02_dev *dev)
if (val & MT_EE_NIC_CONF_0_PA_IO_CURRENT) {
u32 data;
- /* set external external PA I/O
+ /* set external PA I/O
* current to 16mA
*/
data = mt76_rr(dev, 0x11c);
- val |= 0xc03;
- mt76_wr(dev, 0x11c, val);
+ data |= 0xc03;
+ mt76_wr(dev, 0x11c, data);
}
}
^ permalink raw reply related
* Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements
From: Al Viro @ 2018-10-08 6:11 UTC (permalink / raw)
To: David Miller; +Cc: jhs, jiri, xiyou.wangcong, netdev
In-Reply-To: <20181007.225552.488756286647781616.davem@davemloft.net>
On Sun, Oct 07, 2018 at 10:55:52PM -0700, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Mon, 8 Oct 2018 06:45:15 +0100
>
> > Er... Both are due to missing in the very beginning of the series (well, on
> > top of "net: sched: cls_u32: fix hnode refcounting") commit
>
> All dependencies like this must be explicitly stated.
>
> And in such situations you actually should wait for the dependency to
> get into 'net', eventually get merged into 'net-next', and then you
> can submit the stuff that depends upon it.
>
> Not the way this was done.
Point (and this commit is simply missing - it's not that it went into
net). FWIW, this was simply "this is what the breakage is caused by",
not an attempt to amend the submission or anything like that...
^ permalink raw reply
* Re: KMSAN: uninit-value in __dev_mc_add
From: syzbot @ 2018-10-08 13:06 UTC (permalink / raw)
To: davem, edumazet, linux-kernel, netdev, sunlw.fnst, syzkaller-bugs,
vdronov
In-Reply-To: <0000000000005e2e530576c6f9ce@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 43c85fe5a0ee kmsan: suppress false positives in KVM
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=15ffd5b9400000
kernel config: https://syzkaller.appspot.com/x/.config?x=3ff9630e1f32e076
dashboard link: https://syzkaller.appspot.com/bug?extid=001516d86dbe88862cec
compiler: clang version 8.0.0 (trunk 339414)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10adf491400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=100c8159400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+001516d86dbe88862cec@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
IPVS: ftp: loaded support on port[0] = 21
==================================================================
BUG: KMSAN: uninit-value in memcmp+0x117/0x180 lib/string.c:863
CPU: 1 PID: 18 Comm: kworker/1:0 Not tainted 4.19.0-rc4+ #64
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x306/0x460 lib/dump_stack.c:113
kmsan_report+0x1a2/0x2e0 mm/kmsan/kmsan.c:917
__msan_warning+0x7c/0xe0 mm/kmsan/kmsan_instr.c:500
memcmp+0x117/0x180 lib/string.c:863
__hw_addr_add_ex net/core/dev_addr_lists.c:61 [inline]
__dev_mc_add+0x1f9/0x8b0 net/core/dev_addr_lists.c:670
dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687
igmp6_group_added+0x2d7/0xab0 net/ipv6/mcast.c:676
__ipv6_dev_mc_inc+0xeff/0x10f0 net/ipv6/mcast.c:934
ipv6_dev_mc_inc+0x70/0x80 net/ipv6/mcast.c:941
addrconf_join_solict net/ipv6/addrconf.c:2098 [inline]
addrconf_dad_begin net/ipv6/addrconf.c:3879 [inline]
addrconf_dad_work+0x3e7/0x2690 net/ipv6/addrconf.c:4006
process_one_work+0x19c4/0x24f0 kernel/workqueue.c:2153
worker_thread+0x206d/0x2b30 kernel/workqueue.c:2296
kthread+0x59c/0x5d0 kernel/kthread.c:247
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:416
Local variable description: ----buf@igmp6_group_added
Variable was created at:
igmp6_group_added+0x57/0xab0 net/ipv6/mcast.c:664
__ipv6_dev_mc_inc+0xeff/0x10f0 net/ipv6/mcast.c:934
==================================================================
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements
From: David Miller @ 2018-10-08 5:55 UTC (permalink / raw)
To: viro; +Cc: jhs, jiri, xiyou.wangcong, netdev
In-Reply-To: <20181008054515.GC32577@ZenIV.linux.org.uk>
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Mon, 8 Oct 2018 06:45:15 +0100
> Er... Both are due to missing in the very beginning of the series (well, on
> top of "net: sched: cls_u32: fix hnode refcounting") commit
All dependencies like this must be explicitly stated.
And in such situations you actually should wait for the dependency to
get into 'net', eventually get merged into 'net-next', and then you
can submit the stuff that depends upon it.
Not the way this was done.
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements
From: Al Viro @ 2018-10-08 5:45 UTC (permalink / raw)
To: David Miller; +Cc: jhs, jiri, xiyou.wangcong, netdev
In-Reply-To: <20181007.212501.1606549212155525845.davem@davemloft.net>
On Sun, Oct 07, 2018 at 09:25:01PM -0700, David Miller wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> Date: Sun, 7 Oct 2018 12:38:00 -0400
>
> > Various improvements from Al.
>
> Please submit changes that actually are compile tested:
>
> CC [M] net/sched/cls_u32.o
> net/sched/cls_u32.c: In function ‘u32_delete’:
> net/sched/cls_u32.c:674:6: error: ‘root_ht’ undeclared (first use in this function); did you mean ‘root_user’?
> if (root_ht == ht) {
> ^~~~~~~
> root_user
> net/sched/cls_u32.c:674:6: note: each undeclared identifier is reported only once for each function it appears in
> net/sched/cls_u32.c: In function ‘u32_set_parms’:
> net/sched/cls_u32.c:746:15: error: ‘struct tc_u_hnode’ has no member named ‘is_root’
> if (ht_down->is_root) {
> ^~
Er... Both are due to missing in the very beginning of the series (well, on
top of "net: sched: cls_u32: fix hnode refcounting") commit
Author: Al Viro <viro@zeniv.linux.org.uk>
Date: Mon Sep 3 14:39:02 2018 -0400
net: sched: cls_u32: mark root hnode explicitly
... and produce consistent error on attempt to delete such.
Existing check in u32_delete() is inconsistent - after
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 handle 1: u32 divisor 1
tc filter add dev eth0 parent ffff: protocol ip prio 200 handle 2: u32 divisor 1
both
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 801: u32
and
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 800: u32
will fail (at least with refcounting fixes), but the former will complain
about an attempt to remove a busy table, while the latter will recognize
it as root and yield "Not allowed to delete root node" instead.
The problem with the existing check is that several tcf_proto instances might
share the same tp->data and handle-to-hnode lookup will be the same for all
of them. So comparing an hnode to be deleted with tp->root won't catch the
case when one tp is used to try deleting the root of another. Solution is
trivial - mark the root hnodes explicitly upon allocation and check for that.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index b2c3406a2cf2..c4782aa808c7 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -84,6 +84,7 @@ struct tc_u_hnode {
int refcnt;
unsigned int divisor;
struct idr handle_idr;
+ bool is_root;
struct rcu_head rcu;
u32 flags;
/* The 'ht' field MUST be the last field in structure to allow for
@@ -377,6 +378,7 @@ static int u32_init(struct tcf_proto *tp)
root_ht->refcnt++;
root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : 0x80000000;
root_ht->prio = tp->prio;
+ root_ht->is_root = true;
idr_init(&root_ht->handle_idr);
if (tp_c == NULL) {
@@ -693,7 +695,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
goto out;
}
- if (root_ht == ht) {
+ if (ht->is_root) {
NL_SET_ERR_MSG_MOD(extack, "Not allowed to delete root node");
return -EINVAL;
}
^ permalink raw reply related
* [PATCH] isdn/gigaset/isocdata: mark expected switch fall-through
From: Gustavo A. R. Silva @ 2018-10-08 12:15 UTC (permalink / raw)
To: Paul Bolle, Karsten Keil
Cc: gigaset307x-common, netdev, linux-kernel, Gustavo A. R. Silva
Notice that in this particular case, I replaced the
"--v-- fall through --v--" comment with a proper
"fall through", which is what GCC is expecting to
find.
This fix is part of the ongoing efforts to enabling
-Wimplicit-fallthrough
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/isdn/gigaset/isocdata.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/isdn/gigaset/isocdata.c b/drivers/isdn/gigaset/isocdata.c
index 97e00118..f9264ba 100644
--- a/drivers/isdn/gigaset/isocdata.c
+++ b/drivers/isdn/gigaset/isocdata.c
@@ -906,7 +906,7 @@ static void cmd_loop(unsigned char *src, int numbytes, struct inbuf_t *inbuf)
cs->respdata[0] = 0;
break;
}
- /* --v-- fall through --v-- */
+ /* fall through */
case '\r':
/* end of message line, pass to response handler */
if (cbytes >= MAX_RESP_SIZE) {
--
2.7.4
^ permalink raw reply related
* [PATCH net-next] net/ipv6: stop leaking percpu memory in fib6 info
From: Mike Rapoport @ 2018-10-08 12:06 UTC (permalink / raw)
To: David S. Miller; +Cc: David Ahern, netdev, Mike Rapoport, stable
The fib6_info_alloc() function allocates percpu memory to hold per CPU
pointers to rt6_info, but this memory is never freed. Fix it.
Fixes: a64efe142f5e ("net/ipv6: introduce fib6_info struct and helpers")
Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
---
net/ipv6/ip6_fib.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index cf709eadc932..cc7de7eb8b9c 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -194,6 +194,8 @@ void fib6_info_destroy_rcu(struct rcu_head *head)
*ppcpu_rt = NULL;
}
}
+
+ free_percpu(f6i->rt6i_pcpu);
}
lwtstate_put(f6i->fib6_nh.nh_lwtstate);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH V2 net-next 2/5] net: Introduce a new MII time stamping interface.
From: Richard Cochran @ 2018-10-08 4:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, devicetree, David Miller, Florian Fainelli, Jacob Keller,
Mark Rutland, Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20181007212033.3qdtkcjlebsdceus@localhost>
On Sun, Oct 07, 2018 at 02:20:33PM -0700, Richard Cochran wrote:
> On Sun, Oct 07, 2018 at 11:14:05PM +0200, Andrew Lunn wrote:
> > I'm currently thinking register_mii_timestamper() should take a netdev
> > argument, and the net_device structure should gain a struct
> > mii_timestamper.
We are going round in circles on this point. V1 had it this way, but
nobody liked it. You specifically asked to move the new pointer out
of the netdev and into phydev.
> > But we have to look at the lifetime problems. A phydev does not know
> > what netdev it is associated to until phy_connect() is called. It is
> > at that point you can call register_mii_timestamper().
I had used a netdev notifier on NETDEV_UP for this, but Florian seemed
to suggest using phy_{connect,attach,disconnect} instead.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH] net: vhost: remove bad code line
From: David Miller @ 2018-10-08 4:32 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: netdev
In-Reply-To: <1538962910-5765-1-git-send-email-xiangxia.m.yue@gmail.com>
From: xiangxia.m.yue@gmail.com
Date: Sun, 7 Oct 2018 18:41:50 -0700
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Applied.
^ permalink raw reply
* [PATCH] isdn/gigaset: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2018-10-08 11:40 UTC (permalink / raw)
To: Paul Bolle, Karsten Keil
Cc: gigaset307x-common, netdev, linux-kernel, Gustavo A. R. Silva
Replace "--v-- fall through --v--" with a proper "fall through"
annotation. Also, change "bad cid: fall through" to
"fall through - bad cid".
This fix is part of the ongoing efforts to enabling -Wimplicit-fallthrough
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/isdn/gigaset/ev-layer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 1cfcea6..182826e 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -1036,7 +1036,7 @@ static void handle_icall(struct cardstate *cs, struct bc_state *bcs,
break;
default:
dev_err(cs->dev, "internal error: disposition=%d\n", retval);
- /* --v-- fall through --v-- */
+ /* fall through */
case ICALL_IGNORE:
case ICALL_REJECT:
/* hang up actively
@@ -1319,7 +1319,7 @@ static void do_action(int action, struct cardstate *cs,
cs->commands_pending = 1;
break;
}
- /* bad cid: fall through */
+ /* fall through - bad cid */
case ACT_FAILCID:
cs->cur_at_seq = SEQ_NONE;
channel = cs->curchannel;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements
From: David Miller @ 2018-10-08 4:25 UTC (permalink / raw)
To: jhs; +Cc: jiri, xiyou.wangcong, viro, netdev
In-Reply-To: <20181007163811.18453-1-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun, 7 Oct 2018 12:38:00 -0400
> Various improvements from Al.
Please submit changes that actually are compile tested:
CC [M] net/sched/cls_u32.o
net/sched/cls_u32.c: In function ‘u32_delete’:
net/sched/cls_u32.c:674:6: error: ‘root_ht’ undeclared (first use in this function); did you mean ‘root_user’?
if (root_ht == ht) {
^~~~~~~
root_user
net/sched/cls_u32.c:674:6: note: each undeclared identifier is reported only once for each function it appears in
net/sched/cls_u32.c: In function ‘u32_set_parms’:
net/sched/cls_u32.c:746:15: error: ‘struct tc_u_hnode’ has no member named ‘is_root’
if (ht_down->is_root) {
^~
^ permalink raw reply
* Re: [PATCH v8 05/15] octeontx2-af: Add mailbox IRQ and msg handlers
From: David Miller @ 2018-10-08 4:11 UTC (permalink / raw)
To: sunil.kovvuri; +Cc: netdev, arnd, linux-soc, sgoutham
In-Reply-To: <1538924364-18781-6-git-send-email-sunil.kovvuri@gmail.com>
From: sunil.kovvuri@gmail.com
Date: Sun, 7 Oct 2018 20:29:14 +0530
> + req_hdr = (struct mbox_hdr *)(mdev->mbase + mbox->rx_start);
I commented in the previous patch series version, for patch #4, that
such casts are unnecessary and should be removed.
You are in for a very long review process if you only consider feedback
given to you for the specific patch for which it is given, rather than
your entire series.
Please put forth the effort to fix the problems pointed out to you in
your entire series.
Thank you.
^ permalink raw reply
* Re: [PATCH net 1/1] net: sched: cls_u32: fix hnode refcounting
From: David Miller @ 2018-10-08 4:03 UTC (permalink / raw)
To: jhs; +Cc: xiyou.wangcong, jiri, netdev, viro
In-Reply-To: <20181007114017.26087-1-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun, 7 Oct 2018 07:40:17 -0400
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> cls_u32.c misuses refcounts for struct tc_u_hnode - it counts references
> via ->hlist and via ->tp_root together. u32_destroy() drops the former
> and, in case when there had been links, leaves the sucker on the list.
> As the result, there's nothing to protect it from getting freed once links
> are dropped.
> That also makes the "is it busy" check incapable of catching the root
> hnode - it *is* busy (there's a reference from tp), but we don't see it as
> something separate. "Is it our root?" check partially covers that, but
> the problem exists for others' roots as well.
>
> AFAICS, the minimal fix preserving the existing behaviour (where it doesn't
> include oopsen, that is) would be this:
> * count tp->root and tp_c->hlist as separate references. I.e.
> have u32_init() set refcount to 2, not 1.
> * in u32_destroy() we always drop the former;
> in u32_destroy_hnode() - the latter.
>
> That way we have *all* references contributing to refcount. List
> removal happens in u32_destroy_hnode() (called only when ->refcnt is 1)
> an in u32_destroy() in case of tc_u_common going away, along with
> everything reachable from it. IOW, that way we know that
> u32_destroy_key() won't free something still on the list (or pointed to by
> someone's ->root).
>
> Reproducer:
...
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v2 net-next] inet: do not set backlog if listen fails
From: Yafang Shao @ 2018-10-08 10:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David Miller, netdev, LKML
In-Reply-To: <0652963b-90ac-06a7-3a35-50a307465cf9@gmail.com>
On Mon, Oct 8, 2018 at 1:39 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 10/07/2018 06:36 AM, Yafang Shao wrote:
> > We don't need to set the backlog if listen fails.
> > The sk_max_ack_backlog will be set in the caller inet_listen() and
> > dccp_listen_start() if inet_csk_listen_start() return without error.
> > So just remove this line to avoid this unnecessary operation.
> >
> > Regarding sk_ack_backlog, we have to set it before a TCP/DCCP socket is
> > ready to accept new flows to avoid race, because dccp and tcp have lockless
> > listeners
> >
>
> Really could you not add irrelevant stuff in the changelog ?
>
Sure, I will remove it.
> This patch simply removes one redundant setting, you do not have to explain
> about dccp/tcp being lockless and that sk_ack_backlog is not touched by this patch.
>
> Also the title is misleading, since we will still set the backlog even if the listen
> fails.
>
> If you really want sk_max_ack_backlog being not changed if listen() fails,
> then I am afraid you will need to submit a different patch.
After this patch, sk_max_ack_backlog will only be set after the sk is
successfully added to listening hash table,
and then 0 is returned, that means listen() is successful.
Take inet_listen() for example,
if (old_state != TCP_LISTEN) {
err = inet_csk_listen_start(sk, backlog);
if (err)
goto out;
}
// it can only be set if err is 0.
sk->sk_max_ack_backlog = backlog;
err = 0;
out:
return err;
Pls. correct me if I missed something.
Thanks
Yafang
^ permalink raw reply
* [PATCH v2 net-next 22/23] rtnetlink: Move input checking for rtnl_fdb_dump to helper
From: David Ahern @ 2018-10-08 3:16 UTC (permalink / raw)
To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181008031644.15989-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Move the existing input checking for rtnl_fdb_dump into a helper,
valid_fdb_dump_legacy. This function will retain the current
logic that works around the 2 headers that userspace has been
allowed to send up to this point.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/core/rtnetlink.c | 53 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f6d2609cfa9f..c7509c789fb6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3799,22 +3799,13 @@ int ndo_dflt_fdb_dump(struct sk_buff *skb,
}
EXPORT_SYMBOL(ndo_dflt_fdb_dump);
-static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
+static int valid_fdb_dump_legacy(const struct nlmsghdr *nlh,
+ int *br_idx, int *brport_idx,
+ struct netlink_ext_ack *extack)
{
- struct net_device *dev;
+ struct ifinfomsg *ifm = nlmsg_data(nlh);
struct nlattr *tb[IFLA_MAX+1];
- struct net_device *br_dev = NULL;
- const struct net_device_ops *ops = NULL;
- const struct net_device_ops *cops = NULL;
- struct ifinfomsg *ifm = nlmsg_data(cb->nlh);
- struct net *net = sock_net(skb->sk);
- struct hlist_head *head;
- int brport_idx = 0;
- int br_idx = 0;
- int h, s_h;
- int idx = 0, s_idx;
- int err = 0;
- int fidx = 0;
+ int err;
/* A hack to preserve kernel<->userspace interface.
* Before Linux v4.12 this code accepted ndmsg since iproute2 v3.3.0.
@@ -3823,20 +3814,42 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
* Fortunately these sizes don't conflict with the size of ifinfomsg
* with an optional attribute.
*/
- if (nlmsg_len(cb->nlh) != sizeof(struct ndmsg) &&
- (nlmsg_len(cb->nlh) != sizeof(struct ndmsg) +
+ if (nlmsg_len(nlh) != sizeof(struct ndmsg) &&
+ (nlmsg_len(nlh) != sizeof(struct ndmsg) +
nla_attr_size(sizeof(u32)))) {
- err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb,
- IFLA_MAX, ifla_policy, cb->extack);
+ err = nlmsg_parse(nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
+ ifla_policy, extack);
if (err < 0) {
return -EINVAL;
} else if (err == 0) {
if (tb[IFLA_MASTER])
- br_idx = nla_get_u32(tb[IFLA_MASTER]);
+ *br_idx = nla_get_u32(tb[IFLA_MASTER]);
}
- brport_idx = ifm->ifi_index;
+ *brport_idx = ifm->ifi_index;
}
+ return 0;
+}
+
+static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct net_device *dev;
+ struct net_device *br_dev = NULL;
+ const struct net_device_ops *ops = NULL;
+ const struct net_device_ops *cops = NULL;
+ struct net *net = sock_net(skb->sk);
+ struct hlist_head *head;
+ int brport_idx = 0;
+ int br_idx = 0;
+ int h, s_h;
+ int idx = 0, s_idx;
+ int err = 0;
+ int fidx = 0;
+
+ err = valid_fdb_dump_legacy(cb->nlh, &br_idx, &brport_idx,
+ cb->extack);
+ if (err < 0)
+ return err;
if (br_idx) {
br_dev = __dev_get_by_index(net, br_idx);
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 23/23] rtnetlink: Update rtnl_fdb_dump for strict data checking
From: David Ahern @ 2018-10-08 3:16 UTC (permalink / raw)
To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181008031644.15989-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Update rtnl_fdb_dump for strict data checking. If the flag is set,
the dump request is expected to have an ndmsg struct as the header
potentially followed by one or more attributes. Any data passed in the
header or as an attribute is taken as a request to influence the data
returned. Only values supported by the dump handler are allowed to be
non-0 or set in the request. At the moment only the NDA_IFINDEX and
NDA_MASTER attributes are supported.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/core/rtnetlink.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c7509c789fb6..c894c4af8981 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3799,6 +3799,60 @@ int ndo_dflt_fdb_dump(struct sk_buff *skb,
}
EXPORT_SYMBOL(ndo_dflt_fdb_dump);
+static int valid_fdb_dump_strict(const struct nlmsghdr *nlh,
+ int *br_idx, int *brport_idx,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[NDA_MAX + 1];
+ struct ndmsg *ndm;
+ int err, i;
+
+ if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ndm))) {
+ NL_SET_ERR_MSG(extack, "Invalid header for fdb dump request");
+ return -EINVAL;
+ }
+
+ ndm = nlmsg_data(nlh);
+ if (ndm->ndm_pad1 || ndm->ndm_pad2 || ndm->ndm_state ||
+ ndm->ndm_flags || ndm->ndm_type) {
+ NL_SET_ERR_MSG(extack, "Invalid values in header for fbd dump request");
+ return -EINVAL;
+ }
+
+ err = nlmsg_parse_strict(nlh, sizeof(struct ndmsg), tb, NDA_MAX,
+ NULL, extack);
+ if (err < 0)
+ return err;
+
+ *brport_idx = ndm->ndm_ifindex;
+ for (i = 0; i <= NDA_MAX; ++i) {
+ if (!tb[i])
+ continue;
+
+ switch (i) {
+ case NDA_IFINDEX:
+ if (nla_len(tb[i]) != sizeof(u32)) {
+ NL_SET_ERR_MSG(extack, "Invalid IFINDEX attribute in fdb dump request");
+ return -EINVAL;
+ }
+ *brport_idx = nla_get_u32(tb[NDA_IFINDEX]);
+ break;
+ case NDA_MASTER:
+ if (nla_len(tb[i]) != sizeof(u32)) {
+ NL_SET_ERR_MSG(extack, "Invalid MASTER attribute in fdb dump request");
+ return -EINVAL;
+ }
+ *br_idx = nla_get_u32(tb[NDA_MASTER]);
+ break;
+ default:
+ NL_SET_ERR_MSG(extack, "Unsupported attribute in fdb dump request");
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int valid_fdb_dump_legacy(const struct nlmsghdr *nlh,
int *br_idx, int *brport_idx,
struct netlink_ext_ack *extack)
@@ -3846,8 +3900,12 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
int err = 0;
int fidx = 0;
- err = valid_fdb_dump_legacy(cb->nlh, &br_idx, &brport_idx,
- cb->extack);
+ if (cb->strict_check)
+ err = valid_fdb_dump_strict(cb->nlh, &br_idx, &brport_idx,
+ cb->extack);
+ else
+ err = valid_fdb_dump_legacy(cb->nlh, &br_idx, &brport_idx,
+ cb->extack);
if (err < 0)
return err;
--
2.11.0
^ 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