Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/3] dpaa2-eth: Add new statistics counters
From: Jakub Kicinski @ 2019-08-31  6:12 UTC (permalink / raw)
  To: Ioana Radulescu; +Cc: netdev, davem, ioana.ciornei
In-Reply-To: <1567160443-31297-1-git-send-email-ruxandra.radulescu@nxp.com>

On Fri, 30 Aug 2019 13:20:40 +0300, Ioana Radulescu wrote:
> Recent firmware versions offer access to more DPNI statistics
> counters. Add the relevant ones to ethtool interface stats.
> 
> Also we can now make use of a new counter for in flight egress frames
> to avoid sleeping an arbitrary amount of time in the ndo_stop routine.

A little messy there in the comment of patch 2, and IMHO if you're
expecting particular errors to be ignored it's better to write:

	if (err == -EOPNOTSUPP)
		/* still fine*/;
	else if (err)
		/* real err */

than assume any error is for unsupported and add a extra comment
explaining that things may be not supported.

Series LGTM tho.

^ permalink raw reply

* Re: [PATCH net-next 0/2] Minor cleanup in devlink
From: Jakub Kicinski @ 2019-08-31  6:14 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, davem, jiri
In-Reply-To: <20190830103945.18097-1-parav@mellanox.com>

On Fri, 30 Aug 2019 05:39:43 -0500, Parav Pandit wrote:
> Two minor cleanup in devlink.
> 
> Patch-1 Explicitly defines devlink port index as unsigned int
> Patch-2 Uses switch-case to handle different port flavours attributes

Always nice to see one's comment addressed, even if it takes a while :)

Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

^ permalink raw reply

* Re: [PATCH net 0/5] net: aquantia: fixes on vlan filters and other conditions
From: Jakub Kicinski @ 2019-08-31  6:28 UTC (permalink / raw)
  To: Igor Russkikh; +Cc: David S . Miller, netdev@vger.kernel.org
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

On Fri, 30 Aug 2019 12:08:28 +0000, Igor Russkikh wrote:
> Here is a set of various bug fixes related to vlan filter offload and
> two other rare cases.

LGTM, Fixes tag should had been first there on patch 4.

You should also perhaps check the return value from
napi_complete_done() as an optimization for net-next?

^ permalink raw reply

* [PATCH] net: seeq: Fix the function used to release some memory in an error handling path
From: Christophe JAILLET @ 2019-08-31  7:17 UTC (permalink / raw)
  To: davem, yuehaibing, tglx, gregkh, tbogendoerfer
  Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET

In commit 99cd149efe82 ("sgiseeq: replace use of dma_cache_wback_inv"),
a call to 'get_zeroed_page()' has been turned into a call to
'dma_alloc_coherent()'. Only the remove function has been updated to turn
the corresponding 'free_page()' into 'dma_free_attrs()'.
The error hndling path of the probe function has not been updated.

Fix it now.

Rename the corresponding label to something more in line.

Fixes: 99cd149efe82 ("sgiseeq: replace use of dma_cache_wback_inv")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
If 'dma_alloc_coherent()' fails, maybe the message in printk could be
improved. The comment above may also not be relevant.
---
 drivers/net/ethernet/seeq/sgiseeq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/seeq/sgiseeq.c b/drivers/net/ethernet/seeq/sgiseeq.c
index 7a5e6c5abb57..276c7cae7cee 100644
--- a/drivers/net/ethernet/seeq/sgiseeq.c
+++ b/drivers/net/ethernet/seeq/sgiseeq.c
@@ -794,15 +794,16 @@ static int sgiseeq_probe(struct platform_device *pdev)
 		printk(KERN_ERR "Sgiseeq: Cannot register net device, "
 		       "aborting.\n");
 		err = -ENODEV;
-		goto err_out_free_page;
+		goto err_out_free_attrs;
 	}
 
 	printk(KERN_INFO "%s: %s %pM\n", dev->name, sgiseeqstr, dev->dev_addr);
 
 	return 0;
 
-err_out_free_page:
-	free_page((unsigned long) sp->srings);
+err_out_free_attrs:
+	dma_free_attrs(&pdev->dev, sizeof(*sp->srings), sp->srings,
+		       sp->srings_dma, DMA_ATTR_NON_CONSISTENT);
 err_out_free_dev:
 	free_netdev(dev);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH][next] net: hns3: remove redundant assignment to pointer reg_info
From: Colin King @ 2019-08-31  7:29 UTC (permalink / raw)
  To: Yisen Zhuang, Salil Mehta, David S . Miller, Huazhong Tan,
	Zhongzhu Liu, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Pointer reg_info is being initialized with a value that is never read and
is being re-assigned a little later on. The assignment is redundant
and hence can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index 1debe37fe735..1c6b501fb7ca 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -279,7 +279,7 @@ static void hclge_dbg_dump_dcb(struct hclge_dev *hdev, const char *cmd_buf)
 
 static void hclge_dbg_dump_reg_cmd(struct hclge_dev *hdev, const char *cmd_buf)
 {
-	struct hclge_dbg_reg_type_info *reg_info = &hclge_dbg_reg_info[0];
+	struct hclge_dbg_reg_type_info *reg_info;
 	bool has_dump = false;
 	int i;
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next 03/14] bnxt_en: Refactor bnxt_sriov_enable().
From: Leon Romanovsky @ 2019-08-31  7:41 UTC (permalink / raw)
  To: Michael Chan; +Cc: David Miller, Netdev, Vasundhara Volam, Jiri Pirko, Ray Jui
In-Reply-To: <CACKFLiku-5Q6mBFgd2L_gTqZ=UWUf_HTUeC_n6=aVH+V_o1p4g@mail.gmail.com>

On Fri, Aug 30, 2019 at 09:00:27AM -0700, Michael Chan wrote:
> On Fri, Aug 30, 2019 at 2:18 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Mon, Aug 26, 2019 at 09:00:45AM +0300, Leon Romanovsky wrote:
> > > On Sun, Aug 25, 2019 at 11:54:54PM -0400, Michael Chan wrote:
> > > > Refactor the hardware/firmware configuration portion in
> > > > bnxt_sriov_enable() into a new function bnxt_cfg_hw_sriov().  This
> > > > new function can be called after a firmware reset to reconfigure the
> > > > VFs previously enabled.
> > >
> > > I wonder what does it mean for already bound VFs to vfio driver?
> > > Will you rebind them as well? Can I assume that FW error in one VF
> > > will trigger "restart" of other VFs too?
> >
> > Care to reply?
> >
> >
> Sorry, I missed your email earlier.
>
> A firmware reset/recovery has no direct effect on a VF or any function
> if it is just idle.  The PCI interface of any function does not get
> reset.
>
> If a VF driver (Linux VF driver, DPDK driver, etc) has initialized on
> that function, meaning it has exchanged messages with firmware to
> register itself and to allocate resources (such as rings), then the
> firmware reset will require all those resources to be re-discovered
> and re-initialized.  These VF resources are initially assigned by the
> PF.  So this refactored function on the PF is to re-assign these
> resources back to the VF after the firmware reset.  Again, if the VF
> is just bound to vfio and is idle, there is no effect.

Thanks for explaining the flow.

^ permalink raw reply

* Re: [PATCH] net: seeq: Fix the function used to release some memory in an error handling path
From: Thomas Bogendoerfer @ 2019-08-31  8:19 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: davem, yuehaibing, tglx, gregkh, netdev, linux-kernel,
	kernel-janitors
In-Reply-To: <20190831071751.1479-1-christophe.jaillet@wanadoo.fr>

On Sat, 31 Aug 2019 09:17:51 +0200
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:

> In commit 99cd149efe82 ("sgiseeq: replace use of dma_cache_wback_inv"),
> a call to 'get_zeroed_page()' has been turned into a call to
> 'dma_alloc_coherent()'. Only the remove function has been updated to turn
> the corresponding 'free_page()' into 'dma_free_attrs()'.
> The error hndling path of the probe function has not been updated.

Looks good.

Reviewed-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>

Thomas.

-- 
SUSE Software Solutions Germany GmbH
HRB 247165 (AG München)
Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH v5 1/1] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
From: Florian Westphal @ 2019-08-31  8:38 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Roopa Prabhu, Nikolay Aleksandrov, David S. Miller
In-Reply-To: <20190831044032.31931-1-leonardo@linux.ibm.com>

Leonardo Bras <leonardo@linux.ibm.com> wrote:
> A kernel panic can happen if a host has disabled IPv6 on boot and have to
> process guest packets (coming from a bridge) using it's ip6tables.
> 
> IPv6 packets need to be dropped if the IPv6 module is not loaded, and the
> host ip6tables will be used.
> 
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> Changes from v4:
> - Check if the host ip6tables is going to be used before testing 
>   ipv6 module presence 
> - Adds a warning about ipv6 module disabled.
> 
> 
>  net/bridge/br_netfilter_hooks.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index d3f9592f4ff8..af7800103e51 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c
> @@ -496,6 +496,10 @@ static unsigned int br_nf_pre_routing(void *priv,
>  		if (!brnet->call_ip6tables &&
>  		    !br_opt_get(br, BROPT_NF_CALL_IP6TABLES))
>  			return NF_ACCEPT;
> +		if (!ipv6_mod_enabled()) {
> +			pr_warn_once("Module ipv6 is disabled, so call_ip6tables is not supported.");
> +			return NF_DROP;
> +		}

pr_warn_once needs a '\n'.  Pablo, can you mangle this locally when
applying?

Patch looks good to me, so:

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply

* Re: [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
From: Florian Westphal @ 2019-08-31  8:43 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Florian Westphal, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, Pablo Neira Ayuso, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller
In-Reply-To: <2ba876f9ad6597e640df68f09659dce3c4b5ce03.camel@linux.ibm.com>

Leonardo Bras <leonardo@linux.ibm.com> wrote:
> > There are two solutions:
> > 1. The above patch, but use NF_ACCEPT instead
> > 2. keep the DROP, but move it below the call_ip6tables test,
> >    so that users can tweak call-ip6tables to accept packets.
> 
> Q: Does 2 mean that it will only be dropped if bridge intents to use
> host's ip6tables? Else, it will be accepted by previous if?

Yes, thats the idea: Let users decide if ipv6.disable or call-ip6tables
is more important to them.

^ permalink raw reply

* Re: [PATCH v3 01/11] checkpatch: check for nested (un)?likely() calls
From: Markus Elfring @ 2019-08-31  9:15 UTC (permalink / raw)
  To: Denis Efremov, Joe Perches
  Cc: Andrew Morton, Anton Altaparmakov, Andy Whitcroft,
	Boris Ostrovsky, Boris Pismenny, Darrick J. Wong, David S. Miller,
	Dennis Dalessandro, Dmitry Torokhov, dri-devel,
	Inaky Perez-Gonzalez, Jürgen Groß, Leon Romanovsky,
	linux-arm-msm, linux-fsdevel, linux-input, linux-kernel,
	linux-ntfs-dev, linux-rdma, linux-wimax, linux-xfs,
	Mike Marciniszyn, netdev, Pali Rohár, Rob Clark,
	Saeed Mahameed, Sean Paul, Alexander Viro, xen-devel,
	Enrico Weigelt
In-Reply-To: <20190829165025.15750-1-efremov@linux.com>

> +# nested likely/unlikely calls
> +		if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) {
> +			WARN("LIKELY_MISUSE",

How do you think about to use the specification “(?:IS_ERR(?:_(?:OR_NULL|VALUE))?|WARN)”
in this regular expression?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH][next] zd1211rw: zd_usb: Use struct_size() helper
From: kbuild test robot @ 2019-08-31  9:38 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: kbuild-all, Daniel Drake, Ulrich Kunitz, Kalle Valo,
	David S. Miller, linux-wireless, netdev, linux-kernel,
	Gustavo A. R. Silva
In-Reply-To: <20190830185716.GA10044@embeddedor>

[-- Attachment #1: Type: text/plain, Size: 7287 bytes --]

Hi "Gustavo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/zd1211rw-zd_usb-Use-struct_size-helper/20190831-161121
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/net/wireless/zydas/zd1211rw/zd_usb.c:22:0:
   drivers/net/wireless/zydas/zd1211rw/zd_usb.c: In function 'check_read_regs':
>> drivers/net/wireless/zydas/zd1211rw/zd_def.h:18:25: warning: format '%ld' expects argument of type 'long int', but argument 6 has type 'size_t {aka unsigned int}' [-Wformat=]
     dev_printk(level, dev, "%s() " fmt, __func__, ##args)
                            ^
>> drivers/net/wireless/zydas/zd1211rw/zd_def.h:22:4: note: in expansion of macro 'dev_printk_f'
       dev_printk_f(KERN_DEBUG, dev, fmt, ## args)
       ^~~~~~~~~~~~
>> drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1635:3: note: in expansion of macro 'dev_dbg_f'
      dev_dbg_f(zd_usb_dev(usb),
      ^~~~~~~~~
   drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1636:51: note: format string is defined here
        "error: actual length %d less than expected %ld\n",
                                                    ~~^
                                                    %d

vim +/dev_dbg_f +1635 drivers/net/wireless/zydas/zd1211rw/zd_usb.c

e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1622  
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1623  static bool check_read_regs(struct zd_usb *usb, struct usb_req_read_regs *req,
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1624  			    unsigned int count)
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1625  {
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1626  	int i;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1627  	struct zd_usb_interrupt *intr = &usb->intr;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1628  	struct read_regs_int *rr = &intr->read_regs;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1629  	struct usb_int_regs *regs = (struct usb_int_regs *)rr->buffer;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1630  
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1631  	/* The created block size seems to be larger than expected.
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1632  	 * However results appear to be correct.
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1633  	 */
d352eeccec7e84 drivers/net/wireless/zydas/zd1211rw/zd_usb.c Gustavo A. R. Silva 2019-08-30  1634  	if (rr->length < struct_size(regs, regs, count)) {
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02 @1635  		dev_dbg_f(zd_usb_dev(usb),
d352eeccec7e84 drivers/net/wireless/zydas/zd1211rw/zd_usb.c Gustavo A. R. Silva 2019-08-30  1636  			 "error: actual length %d less than expected %ld\n",
d352eeccec7e84 drivers/net/wireless/zydas/zd1211rw/zd_usb.c Gustavo A. R. Silva 2019-08-30  1637  			 rr->length, struct_size(regs, regs, count));
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1638  		return false;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1639  	}
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1640  
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1641  	if (rr->length > sizeof(rr->buffer)) {
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1642  		dev_dbg_f(zd_usb_dev(usb),
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1643  			 "error: actual length %d exceeds buffer size %zu\n",
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1644  			 rr->length, sizeof(rr->buffer));
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1645  		return false;
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1646  	}
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1647  
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1648  	for (i = 0; i < count; i++) {
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1649  		struct reg_data *rd = &regs->regs[i];
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1650  		if (rd->addr != req->addr[i]) {
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1651  			dev_dbg_f(zd_usb_dev(usb),
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1652  				 "rd[%d] addr %#06hx expected %#06hx\n", i,
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1653  				 le16_to_cpu(rd->addr),
e85d0918b54fbd drivers/net/wireless/zd1211rw/zd_usb.c       Daniel Drake        2006-06-02  1654  				 le16_to_cpu(req->addr[i]));
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1655  			return false;
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1656  		}
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1657  	}
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1658  
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1659  	return true;
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1660  }
c900eff30a14ec drivers/net/wireless/zd1211rw/zd_usb.c       Jussi Kivilinna     2011-06-20  1661  

:::::: The code at line 1635 was first introduced by commit
:::::: e85d0918b54fbd9b38003752f7d665416b06edd8 [PATCH] ZyDAS ZD1211 USB-WLAN driver

:::::: TO: Daniel Drake <dsd@gentoo.org>
:::::: CC: Jeff Garzik <jeff@garzik.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61473 bytes --]

^ permalink raw reply

* RE: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: maowenan @ 2019-08-31 10:53 UTC (permalink / raw)
  To: David Miller, cpaasch@apple.com
  Cc: jonathan.lemon@gmail.com, stable@vger.kernel.org,
	gregkh@linuxfoundation.org, tim.froidcoeur@tessares.net,
	matthieu.baerts@tessares.net, aprout@ll.mit.edu,
	edumazet@google.com, jtl@netflix.com,
	linux-kernel@vger.kernel.org, mkubecek@suse.cz,
	ncardwell@google.com, sashal@kernel.org, ycheng@google.com,
	netdev@vger.kernel.org
In-Reply-To: <20190830.192049.1447010488040109227.davem@davemloft.net>

[<ffffff90094930dc>] skb_peek_tail include/linux/skbuff.h:1516 [inline]
[<ffffff90094930dc>] tcp_write_queue_tail include/net/tcp.h:1478 [inline]
[<ffffff90094930dc>] tcp_rtx_queue_tail include/net/tcp.h:1543 [inline]
[<ffffff90094930dc>] tcp_fragment+0xc64/0xce8 net/ipv4/tcp_output.c:1175
[<ffffff90094a37f0>] tcp_write_wakeup+0x3f8/0x4a0 net/ipv4/tcp_output.c:3496
[<ffffff90094a38d0>] tcp_send_probe0+0x38/0x2d8 net/ipv4/tcp_output.c:3523
[<ffffff90094a75a0>] tcp_probe_timer net/ipv4/tcp_timer.c:343 [inline]
[<ffffff90094a75a0>] tcp_write_timer_handler+0x640/0x720 net/ipv4/tcp_timer.c:548
[<ffffff90094a76f8>] tcp_write_timer+0x78/0x1d0 net/ipv4/tcp_timer.c:562
[<ffffff90082610b0>] call_timer_fn.isra.1+0x58/0x180 kernel/time/timer.c:1144
[<ffffff90082616ec>] __run_timers kernel/time/timer.c:1218 [inline]
[<ffffff90082616ec>] run_timer_softirq+0x514/0x848 kernel/time/timer.c:1401
[<ffffff9008141a28>] __do_softirq+0x340/0x878 kernel/softirq.c:273
[<ffffff9008142890>] do_softirq_own_stack include/linux/interrupt.h:498 [inline]
[<ffffff9008142890>] invoke_softirq kernel/softirq.c:357 [inline]
[<ffffff9008142890>] irq_exit+0x170/0x370 kernel/softirq.c:391
[<ffffff900821d550>] __handle_domain_irq+0x100/0x1c0 kernel/irq/irqdesc.c:459
[<ffffff90080914a0>] handle_domain_irq include/linux/irqdesc.h:168 [inline]
[<ffffff90080914a0>] gic_handle_irq+0xd0/0x1f0 drivers/irqchip/irq-gic.c:377

^ permalink raw reply

* Recall: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: maowenan @ 2019-08-31 10:54 UTC (permalink / raw)
  To: David Miller, cpaasch@apple.com
  Cc: jonathan.lemon@gmail.com, stable@vger.kernel.org,
	gregkh@linuxfoundation.org, tim.froidcoeur@tessares.net,
	matthieu.baerts@tessares.net, aprout@ll.mit.edu,
	edumazet@google.com, jtl@netflix.com,
	linux-kernel@vger.kernel.org, mkubecek@suse.cz,
	ncardwell@google.com, sashal@kernel.org, ycheng@google.com,
	netdev@vger.kernel.org

maowenan would like to recall the message, "[PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue".

^ permalink raw reply

* RE: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: maowenan @ 2019-08-31 11:44 UTC (permalink / raw)
  To: David Miller, cpaasch@apple.com
  Cc: jonathan.lemon@gmail.com, stable@vger.kernel.org,
	gregkh@linuxfoundation.org, tim.froidcoeur@tessares.net,
	matthieu.baerts@tessares.net, aprout@ll.mit.edu,
	edumazet@google.com, jtl@netflix.com,
	linux-kernel@vger.kernel.org, mkubecek@suse.cz,
	ncardwell@google.com, sashal@kernel.org, ycheng@google.com,
	netdev@vger.kernel.org
In-Reply-To: <20190830.192049.1447010488040109227.davem@davemloft.net>

Tim 

 Can you share the reproduce steps for this issue? C or syzkaller is ok.
 Thanks a lot.
 

^ permalink raw reply

* Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: Sasha Levin @ 2019-08-31 12:20 UTC (permalink / raw)
  To: Tim Froidcoeur
  Cc: matthieu.baerts, aprout, cpaasch, davem, edumazet, gregkh,
	jonathan.lemon, jtl, linux-kernel, mkubecek, ncardwell, stable,
	ycheng, netdev
In-Reply-To: <20190824060351.3776-1-tim.froidcoeur@tessares.net>

On Sat, Aug 24, 2019 at 08:03:51AM +0200, Tim Froidcoeur wrote:
>Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()")
>triggers following stack trace:
>
>[25244.848046] kernel BUG at ./include/linux/skbuff.h:1406!
>[25244.859335] RIP: 0010:skb_queue_prev+0x9/0xc
>[25244.888167] Call Trace:
>[25244.889182]  <IRQ>
>[25244.890001]  tcp_fragment+0x9c/0x2cf
>[25244.891295]  tcp_write_xmit+0x68f/0x988
>[25244.892732]  __tcp_push_pending_frames+0x3b/0xa0
>[25244.894347]  tcp_data_snd_check+0x2a/0xc8
>[25244.895775]  tcp_rcv_established+0x2a8/0x30d
>[25244.897282]  tcp_v4_do_rcv+0xb2/0x158
>[25244.898666]  tcp_v4_rcv+0x692/0x956
>[25244.899959]  ip_local_deliver_finish+0xeb/0x169
>[25244.901547]  __netif_receive_skb_core+0x51c/0x582
>[25244.903193]  ? inet_gro_receive+0x239/0x247
>[25244.904756]  netif_receive_skb_internal+0xab/0xc6
>[25244.906395]  napi_gro_receive+0x8a/0xc0
>[25244.907760]  receive_buf+0x9a1/0x9cd
>[25244.909160]  ? load_balance+0x17a/0x7b7
>[25244.910536]  ? vring_unmap_one+0x18/0x61
>[25244.911932]  ? detach_buf+0x60/0xfa
>[25244.913234]  virtnet_poll+0x128/0x1e1
>[25244.914607]  net_rx_action+0x12a/0x2b1
>[25244.915953]  __do_softirq+0x11c/0x26b
>[25244.917269]  ? handle_irq_event+0x44/0x56
>[25244.918695]  irq_exit+0x61/0xa0
>[25244.919947]  do_IRQ+0x9d/0xbb
>[25244.921065]  common_interrupt+0x85/0x85
>[25244.922479]  </IRQ>
>
>tcp_rtx_queue_tail() (called by tcp_fragment()) can call
>tcp_write_queue_prev() on the first packet in the queue, which will trigger
>the BUG in tcp_write_queue_prev(), because there is no previous packet.
>
>This happens when the retransmit queue is empty, for example in case of a
>zero window.
>
>Patch is needed for 4.4, 4.9 and 4.14 stable branches.

There needs to be a better explanation of why it's not needed
upstream...

--
Thanks,
Sasha

^ permalink raw reply

* [PATCH v2 net] net: Properly update v4 routes with v6 nexthop
From: Donald Sharp @ 2019-08-31 12:22 UTC (permalink / raw)
  To: netdev, dsahern, sworley

When creating a v4 route that uses a v6 nexthop from a nexthop group.
Allow the kernel to properly send the nexthop as v6 via the RTA_VIA
attribute.

Broken behavior:

$ ip nexthop add via fe80::9 dev eth0
$ ip nexthop show
id 1 via fe80::9 dev eth0 scope link
$ ip route add 4.5.6.7/32 nhid 1
$ ip route show
default via 10.0.2.2 dev eth0
4.5.6.7 nhid 1 via 254.128.0.0 dev eth0
10.0.2.0/24 dev eth0 proto kernel scope link src 10.0.2.15
$

Fixed behavior:

$ ip nexthop add via fe80::9 dev eth0
$ ip nexthop show
id 1 via fe80::9 dev eth0 scope link
$ ip route add 4.5.6.7/32 nhid 1
$ ip route show
default via 10.0.2.2 dev eth0
4.5.6.7 nhid 1 via inet6 fe80::9 dev eth0
10.0.2.0/24 dev eth0 proto kernel scope link src 10.0.2.15
$

v2: Addresses code review comments from David Ahern

Fixes: dcb1ecb50edf (“ipv4: Prepare for fib6_nh from a nexthop object”)
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
 include/net/ip_fib.h     |  4 ++--
 include/net/nexthop.h    |  5 +++--
 net/ipv4/fib_semantics.c | 23 +++++++++++++----------
 net/ipv6/route.c         | 11 ++++++-----
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 4c81846ccce8..ab1ca9e238d2 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -513,7 +513,7 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh,
 			  struct netlink_callback *cb);
 
 int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh,
-		     unsigned char *flags, bool skip_oif);
+		     u8 rt_family, unsigned char *flags, bool skip_oif);
 int fib_add_nexthop(struct sk_buff *skb, const struct fib_nh_common *nh,
-		    int nh_weight);
+		    int nh_weight, u8 rt_family);
 #endif  /* _NET_FIB_H */
diff --git a/include/net/nexthop.h b/include/net/nexthop.h
index 95f766c31c90..331ebbc94fe7 100644
--- a/include/net/nexthop.h
+++ b/include/net/nexthop.h
@@ -161,7 +161,8 @@ struct nexthop *nexthop_mpath_select(const struct nexthop *nh, int nhsel)
 }
 
 static inline
-int nexthop_mpath_fill_node(struct sk_buff *skb, struct nexthop *nh)
+int nexthop_mpath_fill_node(struct sk_buff *skb, struct nexthop *nh,
+			    u8 rt_family)
 {
 	struct nh_group *nhg = rtnl_dereference(nh->nh_grp);
 	int i;
@@ -172,7 +173,7 @@ int nexthop_mpath_fill_node(struct sk_buff *skb, struct nexthop *nh)
 		struct fib_nh_common *nhc = &nhi->fib_nhc;
 		int weight = nhg->nh_entries[i].weight;
 
-		if (fib_add_nexthop(skb, nhc, weight) < 0)
+		if (fib_add_nexthop(skb, nhc, weight, rt_family) < 0)
 			return -EMSGSIZE;
 	}
 
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 2db089e10ba0..673e5f2a6108 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1582,7 +1582,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
 }
 
 int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nhc,
-		     unsigned char *flags, bool skip_oif)
+		     u8 rt_family, unsigned char *flags, bool skip_oif)
 {
 	if (nhc->nhc_flags & RTNH_F_DEAD)
 		*flags |= RTNH_F_DEAD;
@@ -1613,7 +1613,7 @@ int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nhc,
 		/* if gateway family does not match nexthop family
 		 * gateway is encoded as RTA_VIA
 		 */
-		if (nhc->nhc_gw_family != nhc->nhc_family) {
+		if (rt_family != nhc->nhc_gw_family) {
 			int alen = sizeof(struct in6_addr);
 			struct nlattr *nla;
 			struct rtvia *via;
@@ -1654,7 +1654,7 @@ EXPORT_SYMBOL_GPL(fib_nexthop_info);
 
 #if IS_ENABLED(CONFIG_IP_ROUTE_MULTIPATH) || IS_ENABLED(CONFIG_IPV6)
 int fib_add_nexthop(struct sk_buff *skb, const struct fib_nh_common *nhc,
-		    int nh_weight)
+		    int nh_weight, u8 rt_family)
 {
 	const struct net_device *dev = nhc->nhc_dev;
 	struct rtnexthop *rtnh;
@@ -1667,7 +1667,7 @@ int fib_add_nexthop(struct sk_buff *skb, const struct fib_nh_common *nhc,
 	rtnh->rtnh_hops = nh_weight - 1;
 	rtnh->rtnh_ifindex = dev ? dev->ifindex : 0;
 
-	if (fib_nexthop_info(skb, nhc, &flags, true) < 0)
+	if (fib_nexthop_info(skb, nhc, rt_family, &flags, true) < 0)
 		goto nla_put_failure;
 
 	rtnh->rtnh_flags = flags;
@@ -1684,7 +1684,8 @@ EXPORT_SYMBOL_GPL(fib_add_nexthop);
 #endif
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
-static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi)
+static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi,
+			     u8 rt_family)
 {
 	struct nlattr *mp;
 
@@ -1693,13 +1694,14 @@ static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi)
 		goto nla_put_failure;
 
 	if (unlikely(fi->nh)) {
-		if (nexthop_mpath_fill_node(skb, fi->nh) < 0)
+		if (nexthop_mpath_fill_node(skb, fi->nh, rt_family) < 0)
 			goto nla_put_failure;
 		goto mp_end;
 	}
 
 	for_nexthops(fi) {
-		if (fib_add_nexthop(skb, &nh->nh_common, nh->fib_nh_weight) < 0)
+		if (fib_add_nexthop(skb, &nh->nh_common, nh->fib_nh_weight,
+				    rt_family) < 0)
 			goto nla_put_failure;
 #ifdef CONFIG_IP_ROUTE_CLASSID
 		if (nh->nh_tclassid &&
@@ -1717,7 +1719,8 @@ static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi)
 	return -EMSGSIZE;
 }
 #else
-static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi)
+static int fib_add_multipath(struct sk_buff *skb, struct fib_info *fi,
+			     u8 family)
 {
 	return 0;
 }
@@ -1775,7 +1778,7 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
 		const struct fib_nh_common *nhc = fib_info_nhc(fi, 0);
 		unsigned char flags = 0;
 
-		if (fib_nexthop_info(skb, nhc, &flags, false) < 0)
+		if (fib_nexthop_info(skb, nhc, AF_INET, &flags, false) < 0)
 			goto nla_put_failure;
 
 		rtm->rtm_flags = flags;
@@ -1790,7 +1793,7 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
 		}
 #endif
 	} else {
-		if (fib_add_multipath(skb, fi) < 0)
+		if (fib_add_multipath(skb, fi, AF_INET) < 0)
 			goto nla_put_failure;
 	}
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index fd059e08785a..cfb969e68d45 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5329,7 +5329,7 @@ static int rt6_fill_node_nexthop(struct sk_buff *skb, struct nexthop *nh,
 		if (!mp)
 			goto nla_put_failure;
 
-		if (nexthop_mpath_fill_node(skb, nh))
+		if (nexthop_mpath_fill_node(skb, nh, AF_INET6))
 			goto nla_put_failure;
 
 		nla_nest_end(skb, mp);
@@ -5337,7 +5337,7 @@ static int rt6_fill_node_nexthop(struct sk_buff *skb, struct nexthop *nh,
 		struct fib6_nh *fib6_nh;
 
 		fib6_nh = nexthop_fib6_nh(nh);
-		if (fib_nexthop_info(skb, &fib6_nh->nh_common,
+		if (fib_nexthop_info(skb, &fib6_nh->nh_common, AF_INET6,
 				     flags, false) < 0)
 			goto nla_put_failure;
 	}
@@ -5466,13 +5466,14 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
 			goto nla_put_failure;
 
 		if (fib_add_nexthop(skb, &rt->fib6_nh->nh_common,
-				    rt->fib6_nh->fib_nh_weight) < 0)
+				    rt->fib6_nh->fib_nh_weight, AF_INET6) < 0)
 			goto nla_put_failure;
 
 		list_for_each_entry_safe(sibling, next_sibling,
 					 &rt->fib6_siblings, fib6_siblings) {
 			if (fib_add_nexthop(skb, &sibling->fib6_nh->nh_common,
-					    sibling->fib6_nh->fib_nh_weight) < 0)
+					    sibling->fib6_nh->fib_nh_weight,
+					    AF_INET6) < 0)
 				goto nla_put_failure;
 		}
 
@@ -5489,7 +5490,7 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
 
 		rtm->rtm_flags |= nh_flags;
 	} else {
-		if (fib_nexthop_info(skb, &rt->fib6_nh->nh_common,
+		if (fib_nexthop_info(skb, &rt->fib6_nh->nh_common, AF_INET6,
 				     &nh_flags, false) < 0)
 			goto nla_put_failure;
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH net-next] net: hns3: remove set but not used variable 'qos'
From: YueHaibing @ 2019-08-31 12:29 UTC (permalink / raw)
  To: Yisen Zhuang, David S . Miller, Salil Mehta, Yunsheng Lin,
	Zhongzhu Liu, Guangbin Huang, Peng Li, Huazhong Tan, Jian Shen
  Cc: YueHaibing, netdev, kernel-janitors, Hulk Robot

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c: In function 'hclge_restore_vlan_table':
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c:8016:18: warning:
 variable 'qos' set but not used [-Wunused-but-set-variable]

Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 70a214903da9 ("net: hns3: reduce the parameters of some functions")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index ce4b2280a8b0..2b65f2799846 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -8013,7 +8013,7 @@ static void hclge_restore_vlan_table(struct hnae3_handle *handle)
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_vport_vlan_cfg *vlan, *tmp;
 	struct hclge_dev *hdev = vport->back;
-	u16 vlan_proto, qos;
+	u16 vlan_proto;
 	u16 state, vlan_id;
 	int i;
 
@@ -8022,7 +8022,6 @@ static void hclge_restore_vlan_table(struct hnae3_handle *handle)
 		vport = &hdev->vport[i];
 		vlan_proto = vport->port_base_vlan_cfg.vlan_info.vlan_proto;
 		vlan_id = vport->port_base_vlan_cfg.vlan_info.vlan_tag;
-		qos = vport->port_base_vlan_cfg.vlan_info.qos;
 		state = vport->port_base_vlan_cfg.state;
 
 		if (state != HNAE3_PORT_BASE_VLAN_DISABLE) {




^ permalink raw reply related

* [PATCH] net: dsa: Fix off-by-one number of calls to devlink_port_unregister
From: Vladimir Oltean @ 2019-08-31 12:46 UTC (permalink / raw)
  To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean

When a function such as dsa_slave_create fails, currently the following
stack trace can be seen:

[    2.038342] sja1105 spi0.1: Probed switch chip: SJA1105T
[    2.054556] sja1105 spi0.1: Reset switch and programmed static config
[    2.063837] sja1105 spi0.1: Enabled switch tagging
[    2.068706] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy
[    2.076371] ------------[ cut here ]------------
[    2.080973] WARNING: CPU: 1 PID: 21 at net/core/devlink.c:6184 devlink_free+0x1b4/0x1c0
[    2.088954] Modules linked in:
[    2.092005] CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.3.0-rc6-01360-g41b52e38d2b6-dirty #1746
[    2.100912] Hardware name: Freescale LS1021A
[    2.105162] Workqueue: events deferred_probe_work_func
[    2.110287] [<c03133a4>] (unwind_backtrace) from [<c030d8cc>] (show_stack+0x10/0x14)
[    2.117992] [<c030d8cc>] (show_stack) from [<c10b08d8>] (dump_stack+0xb4/0xc8)
[    2.125180] [<c10b08d8>] (dump_stack) from [<c0349d04>] (__warn+0xe0/0xf8)
[    2.132018] [<c0349d04>] (__warn) from [<c0349e34>] (warn_slowpath_null+0x40/0x48)
[    2.139549] [<c0349e34>] (warn_slowpath_null) from [<c0f19d74>] (devlink_free+0x1b4/0x1c0)
[    2.147772] [<c0f19d74>] (devlink_free) from [<c1064fc0>] (dsa_switch_teardown+0x60/0x6c)
[    2.155907] [<c1064fc0>] (dsa_switch_teardown) from [<c1065950>] (dsa_register_switch+0x8e4/0xaa8)
[    2.164821] [<c1065950>] (dsa_register_switch) from [<c0ba7fe4>] (sja1105_probe+0x21c/0x2ec)
[    2.173216] [<c0ba7fe4>] (sja1105_probe) from [<c0b35948>] (spi_drv_probe+0x80/0xa4)
[    2.180920] [<c0b35948>] (spi_drv_probe) from [<c0a4c1cc>] (really_probe+0x108/0x400)
[    2.188711] [<c0a4c1cc>] (really_probe) from [<c0a4c694>] (driver_probe_device+0x78/0x1bc)
[    2.196933] [<c0a4c694>] (driver_probe_device) from [<c0a4a3dc>] (bus_for_each_drv+0x58/0xb8)
[    2.205414] [<c0a4a3dc>] (bus_for_each_drv) from [<c0a4c024>] (__device_attach+0xd0/0x168)
[    2.213637] [<c0a4c024>] (__device_attach) from [<c0a4b1d0>] (bus_probe_device+0x84/0x8c)
[    2.221772] [<c0a4b1d0>] (bus_probe_device) from [<c0a4b72c>] (deferred_probe_work_func+0x84/0xc4)
[    2.230686] [<c0a4b72c>] (deferred_probe_work_func) from [<c03650a4>] (process_one_work+0x218/0x510)
[    2.239772] [<c03650a4>] (process_one_work) from [<c03660d8>] (worker_thread+0x2a8/0x5c0)
[    2.247908] [<c03660d8>] (worker_thread) from [<c036b348>] (kthread+0x148/0x150)
[    2.255265] [<c036b348>] (kthread) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
[    2.262444] Exception stack(0xea965fb0 to 0xea965ff8)
[    2.267466] 5fa0:                                     00000000 00000000 00000000 00000000
[    2.275598] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.283729] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.290333] ---[ end trace ca5d506728a0581a ]---

devlink_free is complaining right here:

	WARN_ON(!list_empty(&devlink->port_list));

This happens because devlink_port_unregister is no longer done right
away in dsa_port_setup when a DSA_PORT_TYPE_USER has failed.
Vivien said about this change that:

    Also no need to call devlink_port_unregister from within dsa_port_setup
    as this step is inconditionally handled by dsa_port_teardown on error.

which is not really true. The devlink_port_unregister function _is_
being called unconditionally from within dsa_port_setup, but not for
this port that just failed, just for the previous ones which were set
up.

ports_teardown:
	for (i = 0; i < port; i++)
		dsa_port_teardown(&ds->ports[i]);

Initially I was tempted to fix this by extending the "for" loop to also
cover the port that failed during setup. But this could have potentially
unforeseen consequences unrelated to devlink_port or even other types of
ports than user ports, which I can't really test for. For example, if
for some reason devlink_port_register itself would fail, then
unconditionally unregistering it in dsa_port_teardown would not be a
smart idea. The list might go on.

So just make dsa_port_setup undo the setup it had done upon failure, and
let the for loop undo the work of setting up the previous ports, which
are guaranteed to be brought up to a consistent state.

Fixes: 955222ca5281 ("net: dsa: use a single switch statement for port setup")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 net/dsa/dsa2.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index f8445fa73448..b501c90aabe4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -259,8 +259,11 @@ static int dsa_port_setup(struct dsa_port *dp)
 	const unsigned char *id = (const unsigned char *)&dst->index;
 	const unsigned char len = sizeof(dst->index);
 	struct devlink_port *dlp = &dp->devlink_port;
+	bool dsa_port_link_registered = false;
+	bool devlink_port_registered = false;
 	struct devlink *dl = ds->devlink;
-	int err;
+	bool dsa_port_enabled = false;
+	int err = 0;
 
 	switch (dp->type) {
 	case DSA_PORT_TYPE_UNUSED:
@@ -272,15 +275,19 @@ static int dsa_port_setup(struct dsa_port *dp)
 				       dp->index, false, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
-			return err;
+			break;
+		devlink_port_registered = true;
 
 		err = dsa_port_link_register_of(dp);
 		if (err)
-			return err;
+			break;
+		dsa_port_link_registered = true;
 
 		err = dsa_port_enable(dp, NULL);
 		if (err)
-			return err;
+			break;
+		dsa_port_enabled = true;
+
 		break;
 	case DSA_PORT_TYPE_DSA:
 		memset(dlp, 0, sizeof(*dlp));
@@ -288,15 +295,19 @@ static int dsa_port_setup(struct dsa_port *dp)
 				       dp->index, false, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
-			return err;
+			break;
+		devlink_port_registered = true;
 
 		err = dsa_port_link_register_of(dp);
 		if (err)
-			return err;
+			break;
+		dsa_port_link_registered = true;
 
 		err = dsa_port_enable(dp, NULL);
 		if (err)
-			return err;
+			break;
+		dsa_port_enabled = true;
+
 		break;
 	case DSA_PORT_TYPE_USER:
 		memset(dlp, 0, sizeof(*dlp));
@@ -304,18 +315,26 @@ static int dsa_port_setup(struct dsa_port *dp)
 				       dp->index, false, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
-			return err;
+			break;
+		devlink_port_registered = true;
 
 		dp->mac = of_get_mac_address(dp->dn);
 		err = dsa_slave_create(dp);
 		if (err)
-			return err;
+			break;
 
 		devlink_port_type_eth_set(dlp, dp->slave);
 		break;
 	}
 
-	return 0;
+	if (err && dsa_port_enabled)
+		dsa_port_disable(dp);
+	if (err && dsa_port_link_registered)
+		dsa_port_link_unregister_of(dp);
+	if (err && devlink_port_registered)
+		devlink_port_unregister(dlp);
+
+	return err;
 }
 
 static void dsa_port_teardown(struct dsa_port *dp)
-- 
2.17.1


^ permalink raw reply related

* iproute2: tc: potential buffer overflow
From: tomaspaukrt @ 2019-08-31 13:13 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 149 bytes --]

Hi,

there are two potentially dangerous calls of strcpy function in the program "tc". In the attachment is a patch that fixes this issue.

Tomas

[-- Attachment #2: iproute2-overflow-fix.patch --]
[-- Type: text/x-diff, Size: 945 bytes --]

diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index cc95eab7..cb64380b 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -269,7 +269,8 @@ static int build_st(struct xtables_target *target, struct ipt_entry_target *t)
 		} else {
 			target->t = t;
 		}
-		strcpy(target->t->u.user.name, target->name);
+		strncpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name) - 1);
 		return 0;
 	}
 
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 6a4509a9..974ac496 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -177,7 +177,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
 	if (t == NULL) {
 		target->t = fw_calloc(1, size);
 		target->t->u.target_size = size;
-		strcpy(target->t->u.user.name, target->name);
+		strncpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name) - 1);
 		set_revision(target->t->u.user.name, target->revision);
 
 		if (target->init != NULL)

^ permalink raw reply related

* Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: Matthieu Baerts @ 2019-08-31 13:14 UTC (permalink / raw)
  To: Sasha Levin, Tim Froidcoeur
  Cc: aprout, cpaasch, davem, edumazet, gregkh, jonathan.lemon, jtl,
	linux-kernel, mkubecek, ncardwell, stable, ycheng, netdev
In-Reply-To: <20190831122036.GY5281@sasha-vm>

Hi Sasha,

Thank you for your reply!

On 31/08/2019 14:20, Sasha Levin wrote:
> On Sat, Aug 24, 2019 at 08:03:51AM +0200, Tim Froidcoeur wrote:
>> Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()")
>> triggers following stack trace:
>>
>> [25244.848046] kernel BUG at ./include/linux/skbuff.h:1406!
>> [25244.859335] RIP: 0010:skb_queue_prev+0x9/0xc
>> [25244.888167] Call Trace:
>> [25244.889182]  <IRQ>
>> [25244.890001]  tcp_fragment+0x9c/0x2cf
>> [25244.891295]  tcp_write_xmit+0x68f/0x988
>> [25244.892732]  __tcp_push_pending_frames+0x3b/0xa0
>> [25244.894347]  tcp_data_snd_check+0x2a/0xc8
>> [25244.895775]  tcp_rcv_established+0x2a8/0x30d
>> [25244.897282]  tcp_v4_do_rcv+0xb2/0x158
>> [25244.898666]  tcp_v4_rcv+0x692/0x956
>> [25244.899959]  ip_local_deliver_finish+0xeb/0x169
>> [25244.901547]  __netif_receive_skb_core+0x51c/0x582
>> [25244.903193]  ? inet_gro_receive+0x239/0x247
>> [25244.904756]  netif_receive_skb_internal+0xab/0xc6
>> [25244.906395]  napi_gro_receive+0x8a/0xc0
>> [25244.907760]  receive_buf+0x9a1/0x9cd
>> [25244.909160]  ? load_balance+0x17a/0x7b7
>> [25244.910536]  ? vring_unmap_one+0x18/0x61
>> [25244.911932]  ? detach_buf+0x60/0xfa
>> [25244.913234]  virtnet_poll+0x128/0x1e1
>> [25244.914607]  net_rx_action+0x12a/0x2b1
>> [25244.915953]  __do_softirq+0x11c/0x26b
>> [25244.917269]  ? handle_irq_event+0x44/0x56
>> [25244.918695]  irq_exit+0x61/0xa0
>> [25244.919947]  do_IRQ+0x9d/0xbb
>> [25244.921065]  common_interrupt+0x85/0x85
>> [25244.922479]  </IRQ>
>>
>> tcp_rtx_queue_tail() (called by tcp_fragment()) can call
>> tcp_write_queue_prev() on the first packet in the queue, which will
>> trigger
>> the BUG in tcp_write_queue_prev(), because there is no previous packet.
>>
>> This happens when the retransmit queue is empty, for example in case of a
>> zero window.
>>
>> Patch is needed for 4.4, 4.9 and 4.14 stable branches.
> 
> There needs to be a better explanation of why it's not needed
> upstream...

Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()") was not a
simple cherry-pick of the original one from master (b617158dc096)
because there is a specific TCP rtx queue only since v4.15. For more
details, please see the commit message of b617158dc096 ("tcp: be more
careful in tcp_fragment()").

The BUG() is hit due to the specific code added to versions older than
v4.15. The comment in skb_queue_prev() (include/linux/skbuff.h:1406),
just before the BUG_ON() somehow suggests to add a check before using
it, what Tim did.

In master, this code path causing the issue will not be taken because
the implementation of tcp_rtx_queue_tail() is different:

    tcp_fragment() → tcp_rtx_queue_tail() → tcp_write_queue_prev() →
skb_queue_prev() → BUG_ON()

Because this patch is specific to versions older than the two last
stable ones but still linked to the network architecture, who can review
and approve it? :)

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts@tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

^ permalink raw reply

* Re: [PATCH 0/4 net-next] flow_offload: update mangle action representation
From: Pablo Neira Ayuso @ 2019-08-31 14:22 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netfilter-devel, davem, netdev, vishal, saeedm, jiri
In-Reply-To: <20190830153351.5d5330fa@cakuba.netronome.com>

On Fri, Aug 30, 2019 at 03:33:51PM -0700, Jakub Kicinski wrote:
> On Fri, 30 Aug 2019 11:07:10 +0200, Pablo Neira Ayuso wrote:
> > > > * The front-end coalesces consecutive pedit actions into one single
> > > >   word, so drivers can mangle IPv6 and ethernet address fields in one
> > > >   single go.  
> > > 
> > > You still only coalesce up to 16 bytes, no?  
> > 
> > You only have to rise FLOW_ACTION_MANGLE_MAXLEN coming in this patch
> > if you need more. I don't know of any packet field larger than 16
> > bytes. If there is a use-case for this, it should be easy to rise that
> > definition.
> 
> Please see the definitions of:
> 
> struct nfp_fl_set_eth
> struct nfp_fl_set_ip4_addrs
> struct nfp_fl_set_ip4_ttl_tos
> struct nfp_fl_set_ipv6_tc_hl_fl
> struct nfp_fl_set_ipv6_addr
> struct nfp_fl_set_tport
> 
> These are the programming primitives for header rewrites in the NFP.
> Since each of those contains more than just one field, we'll have to
> keep all the field coalescing logic in the driver, even if you coalesce
> while fields (i.e. IPv6 addresses).

nfp has been updated in this patch series to deal with the new mangle
representation.

> Perhaps it's not a serious blocker for the series, but it'd be nice if
> rewrite action grouping was handled in the core. Since you're already
> poking at that code..

Rewrite action grouping is already handled from the core front-end in
this patch series.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 2/2] PTP: add support for one-shot output
From: Richard Cochran @ 2019-08-31 14:47 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Christopher S Hall, netdev, linux-kernel, davem
In-Reply-To: <87r253ulpn.fsf@gmail.com>

On Fri, Aug 30, 2019 at 11:00:20AM +0300, Felipe Balbi wrote:
> >> @@ -177,9 +177,8 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
> >>  			err = -EFAULT;
> >>  			break;
> >>  		}
> >> -		if ((req.perout.flags || req.perout.rsv[0] || req.perout.rsv[1]
> >> -				|| req.perout.rsv[2] || req.perout.rsv[3])
> >> -			&& cmd == PTP_PEROUT_REQUEST2) {
> >> +		if ((req.perout.rsv[0] || req.perout.rsv[1] || req.perout.rsv[2]
> >> +			|| req.perout.rsv[3]) && cmd == PTP_PEROUT_REQUEST2) {
> >
> > Please check that the reserved bits of req.perout.flags, namely
> > ~PTP_PEROUT_ONE_SHOT, are clear.
> 
> Actually, we should check more. PEROUT_FEATURE_ENABLE is still valid
> here, right? So are RISING and FALLING edges, no?

No.  The ptp_extts_request.flags are indeed defined:

struct ptp_extts_request {
	...
	unsigned int flags;  /* Bit field for PTP_xxx flags. */
	...
};

But the ptp_perout_request.flags are reserved:

struct ptp_perout_request {
	...
	unsigned int flags;           /* Reserved for future use. */
	...
};

For this ioctl, the test for enable/disable is
ptp_perout_request.period is zero:

		enable = req.perout.period.sec || req.perout.period.nsec;
		err = ops->enable(ops, &req, enable);

The usage pattern here is taken from timer_settime(2).

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH v2 2/2] PTP: add support for one-shot output
From: Richard Cochran @ 2019-08-31 15:01 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Christopher S Hall, netdev, linux-kernel, davem
In-Reply-To: <87r253ulpn.fsf@gmail.com>

On Fri, Aug 30, 2019 at 11:00:20AM +0300, Felipe Balbi wrote:
> seems like this should be defined together with the other flags? If
> that's the case, it seems like we would EXTTS and PEROUT masks.

Yes, let's make the meanings of the bit fields clear...

--- ptp_clock.h ---

/*
 * Bits of the ptp_extts_request.flags field:
 */
#define PTP_ENABLE_FEATURE	BIT(0)
#define PTP_RISING_EDGE		BIT(1)
#define PTP_FALLING_EDGE	BIT(2)
#define PTP_EXTTS_VALID_FLAGS	(PTP_ENABLE_FEATURE | \
				 PTP_RISING_EDGE | \
				 PTP_FALLING_EDGE)

/*
 * Bits of the ptp_perout_request.flags field:
 */
#define PTP_PEROUT_ONE_SHOT	BIT(0)
#define PTP_PEROUT_VALID_FLAGS	(PTP_PEROUT_ONE_SHOT)

struct ptp_extts_request {
	unsigned int flags;  /* Bit field of PTP_EXTTS_VALID_FLAGS. */
};

struct ptp_perout_request {
	unsigned int flags;  /* Bit field of PTP_PEROUT_VALID_FLAGS. */
};


Thanks,
Richard

^ permalink raw reply

* Re: iproute2: tc: potential buffer overflow
From: Stephen Hemminger @ 2019-08-31 15:37 UTC (permalink / raw)
  To: tomaspaukrt; +Cc: netdev
In-Reply-To: <8fo.ZWfD.3kvedbSyU2M.1TQd9t@seznam.cz>

On Sat, 31 Aug 2019 15:13:27 +0200 (CEST)
<tomaspaukrt@email.cz> wrote:

> Hi,
> 
> there are two potentially dangerous calls of strcpy function in the program "tc". In the attachment is a patch that fixes this issue.
> 
> Tomas

This looks correct.

Please fix with strlcpy() instead; that is clearer.
Plus you can use XT_EXTENSION_MAX_NAMELEN here (optional).

^ permalink raw reply

* Re: [PATCH v3 01/11] checkpatch: check for nested (un)?likely() calls
From: Denis Efremov @ 2019-08-31 15:54 UTC (permalink / raw)
  To: Markus Elfring, Joe Perches
  Cc: Andrew Morton, Anton Altaparmakov, Andy Whitcroft,
	Boris Ostrovsky, Boris Pismenny, Darrick J. Wong, David S. Miller,
	Dennis Dalessandro, Dmitry Torokhov, dri-devel,
	Inaky Perez-Gonzalez, Jürgen Groß, Leon Romanovsky,
	linux-arm-msm, linux-fsdevel, linux-input, linux-kernel,
	linux-ntfs-dev, linux-rdma, linux-wimax, linux-xfs,
	Mike Marciniszyn, netdev, Pali Rohár, Rob Clark,
	Saeed Mahameed, Sean Paul, Alexander Viro, xen-devel,
	Enrico Weigelt
In-Reply-To: <0d9345ed-f16a-de0b-6125-1f663765eb46@web.de>



On 31.08.2019 12:15, Markus Elfring wrote:
>> +# nested likely/unlikely calls
>> +        if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) {
>> +            WARN("LIKELY_MISUSE",
> 
> How do you think about to use the specification “(?:IS_ERR(?:_(?:OR_NULL|VALUE))?|WARN)”
> in this regular expression?

Hmm, 
(?:   <- Catch group is required here, since it is used in diagnostic message,
         see $1
   IS_ERR
   (?:_ <- Another atomic group just to show that '_' is a common prefix?
           I'm not sure about this. Usually, Perl interpreter is very good at optimizing such things.
           You could see this optimization if you run perl with -Mre=debug.
     (?:OR_NULL|VALUE))?|WARN)

Regards,
Denis

^ 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