Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/3] ibmvnic: Increase maximum queue size limit
From: Thomas Falcon @ 2018-09-28 23:38 UTC (permalink / raw)
  To: netdev; +Cc: nfont, Thomas Falcon
In-Reply-To: <1538177906-17068-1-git-send-email-tlfalcon@linux.ibm.com>

Increase queue size limit to 16. Devices available for IBM vNIC today
will not allow this amount, but this should give us some leeway for
future devices that may support more RX or TX queues.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index f06eec145ca6..2c1787109f1c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -39,7 +39,7 @@
 #define IBMVNIC_RX_WEIGHT		16
 /* when changing this, update IBMVNIC_IO_ENTITLEMENT_DEFAULT */
 #define IBMVNIC_BUFFS_PER_POOL	100
-#define IBMVNIC_MAX_QUEUES	10
+#define IBMVNIC_MAX_QUEUES	16
 
 #define IBMVNIC_TSO_BUF_SZ	65536
 #define IBMVNIC_TSO_BUFS	64
-- 
2.12.3

^ permalink raw reply related

* [PATCH net-next 0/3] ibmvnic: Implement driver-defined queue limits
From: Thomas Falcon @ 2018-09-28 23:38 UTC (permalink / raw)
  To: netdev; +Cc: nfont, Thomas Falcon

In this patch series, update the ibmvnic driver to use driver-defined
queue limits instead of limits imposed by the Virtual I/O server
management partition. For some deviced, initial max queue size and
amount limits, despite their definition, can actually be exceeded if
the client driver requests it. With this in mind, define a private
ethtool flag that toggles the use of driver-defined limits. These
limits are currently more than what supported hardware will likely
allow, so the driver will attempt to get as close as possible to
the user request but may not fully succeed.

Thomas Falcon (3):
  ibmvnic: Increase maximum queue size limit
  ibmvnic: Introduce driver limits for ring sizes
  ibmvnic: Add ethtool private flag for driver-defined queue limits

 drivers/net/ethernet/ibm/ibmvnic.c | 129 +++++++++++++++++++++++++++----------
 drivers/net/ethernet/ibm/ibmvnic.h |   9 ++-
 2 files changed, 102 insertions(+), 36 deletions(-)

-- 
2.12.3

^ permalink raw reply

* RE: [PATCH net v2 3/7] lan78xx: Check for supported Wake-on-LAN modes
From: Woojung.Huh @ 2018-09-28 23:27 UTC (permalink / raw)
  To: f.fainelli, netdev
  Cc: davem, UNGLinuxDriver, steve.glendinning, keescook, akurz,
	hayeswang, kai.heng.feng, grundler, zhongjiang, bigeasy,
	ran.wang_1, edumazet, linux-usb, linux-kernel
In-Reply-To: <20180928231856.3587-4-f.fainelli@gmail.com>

> The driver supports a fair amount of Wake-on-LAN modes, but is not
> checking that the user specified one that is supported.
> 
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Woojung Huh <Woojung.Huh@Microchip.com>

Thanks.
Woojung

^ permalink raw reply

* Re: [PATCH net v2] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Vijay Khemka @ 2018-09-28 23:22 UTC (permalink / raw)
  To: Justin.Lee1@Dell.com, joel@jms.id.au, sam@mendozajonas.com
  Cc: linux-aspeed@lists.ozlabs.org, netdev@vger.kernel.org,
	openbmc@lists.ozlabs.org, Amithash Prasad, christian@cmd.nu
In-Reply-To: <58fff61ffd8d4038b59f32b696e76bbc@AUSX13MPS306.AMER.DELL.COM>



>On 9/28/18, 11:16 AM, "Justin.Lee1@Dell.com" <Justin.Lee1@Dell.com> wrote:

  >  The new command (NCSI_CMD_SEND_CMD) is added to allow user space application 
  >  to send NC-SI command to the network card.
  >  Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
  > 
  >  The work flow is as below. 
  >  
  >  Request:
  >  User space application -> Netlink interface (msg)
                                                  -> new Netlink handler - ncsi_send_cmd_nl()
                                                  -> ncsi_xmit_cmd()
  >  Response:
  >  Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx()
                                                                            -> ncsi_rsp_handler_netlink()
                                                                            -> ncsi_send_netlink_rsp ()
                                                                            -> Netlink interface (msg)
                                                                            -> user space application
  >  Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
                                                                                                -> Netlink interface (msg with zero data length)
                                                                                                -> user space application
  >  Error:
  >  Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
                                                                                           -> user space application
    
    
    I will request to keep 2 patch, one for OEM handler and other one for netlink user space handling.


^ permalink raw reply

* [PATCH net v2 6/7] smsc75xx: Check for Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Woojung Huh,
	Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
	Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
	zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
	open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>

The driver does not check for Wake-on-LAN modes specified by an user,
but will conditionally set the device as wake-up enabled or not based on
that, which could be a very confusing user experience.

Fixes: 6c636503260d ("smsc75xx: add wol magic packet support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/usb/smsc75xx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 05553d252446..e5a4cbb366dc 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -731,6 +731,9 @@ static int smsc75xx_ethtool_set_wol(struct net_device *net,
 	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
 	int ret;
 
+	if (wolinfo->wolopts & ~SUPPORTED_WAKE)
+		return -EINVAL;
+
 	pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
 
 	ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
-- 
2.17.1

^ permalink raw reply related

* [PATCH net v2 4/7] sr9800: Check for supported Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Woojung Huh,
	Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
	Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
	zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
	open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>

The driver currently silently accepts unsupported Wake-on-LAN modes
(other than WAKE_PHY or WAKE_MAGIC) without reporting that to the user,
which is confusing.

Fixes: 19a38d8e0aa3 ("USB2NET : SR9800 : One chip USB2.0 USB2NET SR9800 Device Driver Support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/usb/sr9800.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 9277a0f228df..35f39f23d881 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -421,6 +421,9 @@ sr_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
 	struct usbnet *dev = netdev_priv(net);
 	u8 opt = 0;
 
+	if (wolinfo->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
+		return -EINVAL;
+
 	if (wolinfo->wolopts & WAKE_PHY)
 		opt |= SR_MONITOR_LINK;
 	if (wolinfo->wolopts & WAKE_MAGIC)
-- 
2.17.1

^ permalink raw reply related

* [PATCH net v2 1/7] asix: Check for supported Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Woojung Huh,
	Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
	Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
	zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
	open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>

The driver currently silently accepts unsupported Wake-on-LAN modes
(other than WAKE_PHY or WAKE_MAGIC) without reporting that to the user,
which is confusing.

Fixes: 2e55cc7210fe ("[PATCH] USB: usbnet (3/9) module for ASIX Ethernet adapters")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/usb/asix_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index e95dd12edec4..023b8d0bf175 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -607,6 +607,9 @@ int asix_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
 	struct usbnet *dev = netdev_priv(net);
 	u8 opt = 0;
 
+	if (wolinfo->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
+		return -EINVAL;
+
 	if (wolinfo->wolopts & WAKE_PHY)
 		opt |= AX_MONITOR_LINK;
 	if (wolinfo->wolopts & WAKE_MAGIC)
-- 
2.17.1

^ permalink raw reply related

* [PATCH] net: phy: Fix a unused function gcc warning.
From: zhong jiang @ 2018-09-29  5:11 UTC (permalink / raw)
  To: davem; +Cc: f.fainelli, andrew, netdev, linux-kernel

Fix the following compile warning:

drivers/net/phy/mdio-bcm-unimac.c:321:12: warning: 'unimac_mdio_suspend' defined but not used [-Wunused-function]
 static int unimac_mdio_suspend(struct device *d)
            ^
drivers/net/phy/mdio-bcm-unimac.c:330:12: warning: 'unimac_mdio_resume' defined but not used [-Wunused-function]
 static int unimac_mdio_resume(struct device *d)

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/phy/mdio-bcm-unimac.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/mdio-bcm-unimac.c b/drivers/net/phy/mdio-bcm-unimac.c
index 80b9583..f5b83ea 100644
--- a/drivers/net/phy/mdio-bcm-unimac.c
+++ b/drivers/net/phy/mdio-bcm-unimac.c
@@ -318,6 +318,7 @@ static int unimac_mdio_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
 static int unimac_mdio_suspend(struct device *d)
 {
 	struct unimac_mdio_priv *priv = dev_get_drvdata(d);
@@ -343,6 +344,7 @@ static int unimac_mdio_resume(struct device *d)
 
 static SIMPLE_DEV_PM_OPS(unimac_mdio_pm_ops,
 			 unimac_mdio_suspend, unimac_mdio_resume);
+#endif /* CONFIG_PM_SLEEP */
 
 static const struct of_device_id unimac_mdio_ids[] = {
 	{ .compatible = "brcm,genet-mdio-v5", },
@@ -359,7 +361,9 @@ static SIMPLE_DEV_PM_OPS(unimac_mdio_pm_ops,
 	.driver = {
 		.name = UNIMAC_MDIO_DRV_NAME,
 		.of_match_table = unimac_mdio_ids,
+#ifdef CONFIG_PM_SLEEP
 		.pm = &unimac_mdio_pm_ops,
+#endif
 	},
 	.probe	= unimac_mdio_probe,
 	.remove	= unimac_mdio_remove,
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
From: Vijay Khemka @ 2018-09-28 22:26 UTC (permalink / raw)
  To: Samuel Mendoza-Jonas, linux-aspeed @ lists . ozlabs . org,
	openbmc @ lists . ozlabs . org, Sai Dasari, Amithash Prasad,
	Justin.Lee1@dell.com, netdev@vger.kernel.org
In-Reply-To: <81718e3fd6c883917aca540c032a7065fd00e79a.camel@mendozajonas.com>



On 9/26/18, 8:44 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:

>    Hi Vijay,
    
>    Having had a chance to take a closer look, there is probably room for
>   both this patchset and Justin's potential changes to coexist; while
>  Justin's is a more general solution for sending arbitrary commands, the
>    approach of this patch is also useful for handling commands we want
>    included in the configure process (such as get-mac-address).

Hi Sam,
I have created a more generic approach based patch, will send you after clean up. I agree we need both Justin patch as well as mine to handle OEM specific command. I am creating 2 patches one for generic OEM command and response handling and another is OEM vendor specific command handling. Please see my responses to your comments below.
    
    Some comments below:
    
    > ---
    >  net/ncsi/Kconfig       |  3 ++
    >  net/ncsi/internal.h    | 11 +++++--
    >  net/ncsi/ncsi-cmd.c    | 24 +++++++++++++--
    >  net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++
    >  net/ncsi/ncsi-pkt.h    | 16 ++++++++++
    >  net/ncsi/ncsi-rsp.c    | 33 +++++++++++++++++++-
    >  6 files changed, 149 insertions(+), 6 deletions(-)
    > 
    > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
    > index 08a8a6031fd7..b8bf89fea7c8 100644
    > --- a/net/ncsi/Kconfig
    > +++ b/net/ncsi/Kconfig
    > @@ -10,3 +10,6 @@ config NET_NCSI
    >  	  support. Enable this only if your system connects to a network
    >  	  device via NCSI and the ethernet driver you're using supports
    >  	  the protocol explicitly.
    > +config NCSI_OEM_CMD_GET_MAC
    > +	bool "Get NCSI OEM MAC Address"
    > +	depends on NET_NCSI
    
        For the moment this isn't too bad but I wonder if in the future it would
        be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we
        could selectively enable a class of OEM commands based on vendor rather
        than per-command.
    Certainly, we can have like that and will be an addition to above config. 
    Above config is to enable retrieving and setting mac address from device 
    during channel configuration. And then we can have vendor selection like 
    MELLANOX, Broadcom etc. But I will rather check GV ID under this config 
    and see if there is available function for specific vendor. This can be revised
    and made more generic based on vendor.

    > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
    > index 8055e3965cef..da17958e6a4b 100644
    > --- a/net/ncsi/internal.h
    > +++ b/net/ncsi/internal.h
    > @@ -68,6 +68,10 @@ enum {
    >  	NCSI_MODE_MAX
    >  };
    >  
    > +#define NCSI_OEM_MFR_MLX_ID             0x8119
    > +#define NCSI_OEM_MLX_CMD_GET_MAC        0x1b00
    > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY   0x010700
    
        I gather this is part of the OEM command but it would be good to describe
        what these bits mean. Is this command documented anywhere by Mellanox?
    I will add more description here, please see in next patch. Yes Mellanox 
    specification describes these commands.

    > +
    >  struct ncsi_channel_version {
    >  	u32 version;		/* Supported BCD encoded NCSI version */
    >  	u32 alpha2;		/* Supported BCD encoded NCSI version */
    > @@ -236,6 +240,7 @@ enum {
    >  	ncsi_dev_state_probe_dp,
    >  	ncsi_dev_state_config_sp	= 0x0301,
    >  	ncsi_dev_state_config_cis,
    > +	ncsi_dev_state_config_oem_gma,
    >  	ncsi_dev_state_config_clear_vids,
    >  	ncsi_dev_state_config_svf,
    >  	ncsi_dev_state_config_ev,
    > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
    >  	unsigned short       payload;     /* Command packet payload length */
    >  	unsigned int         req_flags;   /* NCSI request properties       */
    >  	union {
    > -		unsigned char  bytes[16]; /* Command packet specific data  */
    > -		unsigned short words[8];
    > -		unsigned int   dwords[4];
    > +		unsigned char  bytes[64]; /* Command packet specific data  */
    > +		unsigned short words[32];
    > +		unsigned int   dwords[16];
    >  	};
    >  };
    >  
    > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
    > index 7567ca63aae2..3205e22c1734 100644
    > --- a/net/ncsi/ncsi-cmd.c
    > +++ b/net/ncsi/ncsi-cmd.c
    > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
    >  	return 0;
    >  }
    >  
    > +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
    > +				struct ncsi_cmd_arg *nca)
    > +{
    > +	struct ncsi_cmd_oem_pkt *cmd;
    > +	unsigned int len;
    > +
    > +	len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
    > +	if (nca->payload < 26)
    > +		len += 26;
    
        This will have already happened in ncsi_alloc_command(), is this check
        needed?
    Yes it is needed to find length for skbuff data to initialize. 
    ncsi_alloc_command() does the same and allocate skbuff.  

    > +	else
    > +		len += nca->payload;
    > +
    > +	cmd = skb_put_zero(skb, len);
    > +	memcpy(cmd->data, nca->bytes, nca->payload);
    > +	ncsi_cmd_build_header(&cmd->cmd.common, nca);
    > +
    > +	return 0;
    > +}
    > +
    >  static struct ncsi_cmd_handler {
    >  	unsigned char type;
    >  	int           payload;
    > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
    >  	{ NCSI_PKT_CMD_GNS,    0, ncsi_cmd_handler_default },
    >  	{ NCSI_PKT_CMD_GNPTS,  0, ncsi_cmd_handler_default },
    >  	{ NCSI_PKT_CMD_GPS,    0, ncsi_cmd_handler_default },
    > -	{ NCSI_PKT_CMD_OEM,    0, NULL                     },
    > +	{ NCSI_PKT_CMD_OEM,   -1, ncsi_cmd_handler_oem     },
    >  	{ NCSI_PKT_CMD_PLDM,   0, NULL                     },
    >  	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
    >  };
    > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
    >  	}
    >  
    >  	/* Get packet payload length and allocate the request */
    > -	nca->payload = nch->payload;
    > +	if (nch->payload >= 0)
    > +		nca->payload = nch->payload;
    
        I think with this there is a chance of nca->payload being uninitialised
        and then used in ncsi_alloc_command(). Can you describe what the aim here
        is?
    I will add more description here.

    >  	nr = ncsi_alloc_command(nca);
    >  	if (!nr)
    >  		return -ENOMEM;
    > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
    > index 091284760d21..3b2b86560cc8 100644
    > --- a/net/ncsi/ncsi-manage.c
    > +++ b/net/ncsi/ncsi-manage.c
    > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
    >  	return 0;
    >  }
    >  
    > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
    > +/* NCSI Facebook OEM APIs */
    
        Are these Facebook OEM commands or Mellanox OEM commands?
    Will be taken care in next patch

    > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp)
    > +{
    > +	struct ncsi_cmd_arg nca;
    > +	int ret = 0;
    > +
    > +	memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
    > +	nca.ndp = ndp;
    > +	nca.channel = ndp->active_channel->id;
    > +	nca.package = ndp->active_package->id;
    > +	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
    > +	nca.type = NCSI_PKT_CMD_OEM;
    > +	nca.payload = 8;
    > +
    > +	nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
    > +	nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC);
    > +
    > +	ret = ncsi_xmit_cmd(&nca);
    > +	if (ret)
    > +		netdev_err(ndp->ndev.dev,
    > +			   "NCSI: Failed to transmit cmd 0x%x during probe\n",
    > +			   nca.type);
    > +}
    > +
    > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp)
    > +{
    > +	struct ncsi_cmd_arg nca;
    > +	int ret = 0;
    > +
    > +	memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
    
        Ah I see we initialise nca, and thus payload here - the path between
        these two points in the driver isn't super obvious (not this patch's
        fault :) ), it might be better to explicitly mention/handle this where we
        use payload later on.
    Yes, I agree, will add more details.

    > +	nca.ndp = ndp;
    > +	nca.channel = ndp->active_channel->id;
    > +	nca.package = ndp->active_package->id;
    
        These should probably be set in ncsi_configure_channel (eg. see the CIS
        state before these are called).
    These functions are being called from ncsi_configure_channel() only. We 
    can either initialize nca there in ncsi_configure_channel() function and pass 
    nca as function parameter or initialize nca inside function and populate 
    required field.

    > +	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
    > +	nca.type = NCSI_PKT_CMD_OEM;
    > +	nca.payload = 60;
    > +
    > +	nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
    > +	nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY);
    > +
    > +	memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN);
    > +	nca.bytes[14] = 0x09;
    > +
    > +	ret = ncsi_xmit_cmd(&nca);
    > +	if (ret)
    > +		netdev_err(ndp->ndev.dev,
    > +			   "NCSI: Failed to transmit cmd 0x%x during probe\n",
    > +				   nca.type);
    > +}
    > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
    > +
    >  static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
    >  {
    >  	struct ncsi_dev *nd = &ndp->ndev;
    > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
    >  			goto error;
    >  		}
    >  
    > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
    > +		/* Check Manufacture id if it is Mellanox then
    > +		 * get and set mac address. To Do: Add code for
    > +		 * other types of card if required
    > +		 */
    > +		if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID)
    > +			nd->state = ncsi_dev_state_config_oem_gma;
    > +		else
    > +			nd->state = ncsi_dev_state_config_clear_vids;
    > +		break;
    > +	case ncsi_dev_state_config_oem_gma:
    > +		ndp->pending_req_num = 2;
    > +		get_mac_address_oem_mlx(ndp);
    > +		msleep(500);
    
        Is this msleep() required?
    It is required to make sure previous command completed. 
    Will try to handle it differently

    > +		set_mac_affinity_mlx(ndp);
    
        Since this *sets* the MAC address, should we name the state and config
        option more accurately? How does this OEM command interact with the NCSI
        set-mac-address command?
        Does this command depend on the previous get_mac_address_oem_mlx()
        command succeeding?
    It is independent of Set_mac_address. Yes it depends on succession 
    of get_mac_address_oem_mlx() that's why msleep was put there.
    
    > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
    >  		nd->state = ncsi_dev_state_config_clear_vids;
    >  		break;
    >  	case ncsi_dev_state_config_clear_vids:
    > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
    > index 91b4b66438df..0653a893eb12 100644
    > --- a/net/ncsi/ncsi-pkt.h
    > +++ b/net/ncsi/ncsi-pkt.h
    > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
    >  	unsigned char           pad[22];
    >  };
    >  
    > +/* Oem Request Command */
    
        In general, s/Oem/OEM
    Sure, will be done.

    > +struct ncsi_cmd_oem_pkt {
    > +	struct ncsi_cmd_pkt_hdr cmd;         /* Command header    */
    > +	unsigned char           data[64];    /* OEM Payload Data  */
    > +	__be32                  checksum;    /* Checksum          */
    > +};
    > +
    > +/* Oem Response Packet */
    > +struct ncsi_rsp_oem_pkt {
    > +	struct ncsi_rsp_pkt_hdr rsp;         /* Command header    */
    > +	__be32                  mfr_id;      /* Manufacture ID    */
    > +	__be32                  oem_cmd;     /* oem command       */
    > +	unsigned char           data[32];    /* Payload data      */
    > +	__be32                  checksum;    /* Checksum          */
    > +};
    > +
    >  /* Get Link Status */
    >  struct ncsi_rsp_gls_pkt {
    >  	struct ncsi_rsp_pkt_hdr rsp;        /* Response header   */
    > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
    > index 930c1d3796f0..3b94c96b9c7f 100644
    > --- a/net/ncsi/ncsi-rsp.c
    > +++ b/net/ncsi/ncsi-rsp.c
    > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
    >  	return 0;
    >  }
    >  
    > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
    > +{
    > +	struct ncsi_rsp_oem_pkt *rsp;
    > +	struct ncsi_dev_priv *ndp = nr->ndp;
    > +	struct net_device *ndev = ndp->ndev.dev;
    > +	int ret = 0;
    > +	unsigned int oem_cmd, mfr_id;
    > +	const struct net_device_ops *ops = ndev->netdev_ops;
    > +	struct sockaddr saddr;
    > +
    > +	/* Get the response header */
    > +	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
    > +
    > +	oem_cmd = ntohl(rsp->oem_cmd);
    > +	mfr_id = ntohl(rsp->mfr_id);
    > +
    > +	/* Check for Mellanox manufacturer id */
    > +	if (mfr_id != NCSI_OEM_MFR_MLX_ID)
    > +		return 0;
    > +
    > +	if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) {
    > +		saddr.sa_family = ndev->type;
    > +		ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
    > +		memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN);
    > +		ret = ops->ndo_set_mac_address(ndev, &saddr);
    > +		if (ret < 0)
    > +			netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
    > +	}
    > +	return ret;
    > +}
    > +
    >  static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
    >  {
    >  	struct ncsi_rsp_gvi_pkt *rsp;
    > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler {
    >  	{ NCSI_PKT_RSP_GNS,   172, ncsi_rsp_handler_gns     },
    >  	{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts   },
    >  	{ NCSI_PKT_RSP_GPS,     8, ncsi_rsp_handler_gps     },
    > -	{ NCSI_PKT_RSP_OEM,     0, NULL                     },
    > +	{ NCSI_PKT_RSP_OEM,    -1, ncsi_rsp_handler_oem     },
    >  	{ NCSI_PKT_RSP_PLDM,    0, NULL                     },
    >  	{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid  }
    >  };
    
    
    


^ permalink raw reply

* Bad MAINTAINERS pattern in section 'MARVELL 88E6XXX ETHERNET SWITCH FABRIC DRIVER'
From: Joe Perches @ 2018-09-28 21:54 UTC (permalink / raw)
  To: linux-kernel; +Cc: Andrew Lunn, Vivien Didelot, netdev

Please fix this defect appropriately.

linux-next MAINTAINERS section:

	8726	MARVELL 88E6XXX ETHERNET SWITCH FABRIC DRIVER
	8727	M:	Andrew Lunn <andrew@lunn.ch>
	8728	M:	Vivien Didelot <vivien.didelot@savoirfairelinux.com>
	8729	L:	netdev@vger.kernel.org
	8730	S:	Maintained
	8731	F:	drivers/net/dsa/mv88e6xxx/
-->	8732	F:	linux/platform_data/mv88e6xxx.h
	8733	F:	Documentation/devicetree/bindings/net/dsa/marvell.txt

Commit that introduced this:

commit 877b7cb0b6f283593a663134ee52703f12c895cc
 Author: Andrew Lunn <andrew@lunn.ch>
 Date:   Sat May 19 22:31:34 2018 +0200
 
     net: dsa: mv88e6xxx: Add minimal platform_data support
     
     Not all the world uses device tree. Some parts of the world still use
     platform devices and platform data. Add basic support for probing a
     Marvell switch via platform data.
     
     Signed-off-by: Andrew Lunn <andrew@lunn.ch>
     Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
     Signed-off-by: David S. Miller <davem@davemloft.net>
 
  MAINTAINERS                             |  1 +
  drivers/net/dsa/mv88e6xxx/chip.c        | 56 ++++++++++++++++++++++++++++-----
  include/linux/platform_data/mv88e6xxx.h | 17 ++++++++++
  3 files changed, 67 insertions(+), 7 deletions(-)

No commit with linux/platform_data/mv88e6xxx.h found

^ permalink raw reply

* Re: [net] r8169: fix network stalls due to missing bit TXCFG_AUTO_FIFO
From: Heiner Kallweit @ 2018-09-28 21:52 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org, Tony Atkinson, David Arendt,
	Ortwin Glück
In-Reply-To: <7c59d5a8-44b6-3cf5-3321-7201f67305c8@maciej.szmigiero.name>

On 28.09.2018 23:47, Maciej S. Szmigiero wrote:
> On 28.09.2018 22:19, Heiner Kallweit wrote:
>> Some of the chip-specific hw_start functions set bit TXCFG_AUTO_FIFO
>> in register TxConfig. The original patch changed the order of some
>> calls resulting in these changes being overwritten by
>> rtl_set_tx_config_registers() in rtl_hw_start(). This eventually
>> resulted in network stalls especially under high load.
>>
>> Analyzing the chip-specific hw_start functions all chip version from
>> 34, with the exception of version 39, need this bit set.
>> This patch moves setting this bit to rtl_set_tx_config_registers().
>>
>> Fixes: 4fd48c4ac0a0 ("r8169: move common initializations to tp->hw_start")
>> Reported-by: Ortwin Glück <odi@odi.ch>
>> Reported-by: David Arendt <admin@prnet.org>
>> Tested-by: Tony Atkinson <tatkinson@linux.com>
>> Tested-by: David Arendt <admin@prnet.org>
>> Tested-by: Ortwin Glück <odi@odi.ch>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Please add:
> Root-caused-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> 
Sure, forgot you in the list.

Heiner

> Thanks,
> Maciej
> 

^ permalink raw reply

* Re: [PATCH net-next] tcp/fq: move back to CLOCK_MONOTONIC
From: Eric Dumazet @ 2018-09-28 21:52 UTC (permalink / raw)
  To: Leonard Crestez, davem@davemloft.net, edumazet@google.com
  Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <2e9b1489d291cd0194eb0d65ad8a0e96eb36d5f9.camel@nxp.com>



On 09/28/2018 02:27 PM, Leonard Crestez wrote:
> On Fri, 2018-09-28 at 10:28 -0700, Eric Dumazet wrote:
>> In the recent TCP/EDT patch series, I switched TCP and sch_fq
>> clocks from MONOTONIC to TAI, in order to meet the choice done
>> earlier for sch_etf packet scheduler.
>>
>> But sure enough, this broke some setups were the TAI clock
>> jumps forward (by almost 50 year...), as reported
>> by Leonard Crestez.
>>
>> If we want to converge later, we'll probably need to add
>> an skb field to differentiate the clock bases, or a socket option.
>>
>> In the meantime, an UDP application will need to use CLOCK_MONOTONIC
>> base for its SCM_TXTIME timestamps if using fq packet scheduler.
>>
>> Fixes: 72b0094f9182 ("tcp: switch tcp_clock_ns() to CLOCK_TAI base")
>> Fixes: 142537e41923 ("net_sched: sch_fq: switch to CLOCK_TAI")
>> Fixes: fd2bca2aa789 ("tcp: switch internal pacing timer to CLOCK_TAI")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
> 
> Tested-by: Leonard Crestez <leonard.crestez@nxp.com>
> 
> Fixes the problem reported earlier when applied on top of next-20180928
> 

Thanks again ;)

^ permalink raw reply

* [PATCH net v2] r8169: fix network stalls due to missing bit TXCFG_AUTO_FIFO
From: Heiner Kallweit @ 2018-09-28 21:51 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Tony Atkinson, David Arendt,
	Ortwin Glück

Some of the chip-specific hw_start functions set bit TXCFG_AUTO_FIFO
in register TxConfig. The original patch changed the order of some
calls resulting in these changes being overwritten by
rtl_set_tx_config_registers() in rtl_hw_start(). This eventually
resulted in network stalls especially under high load.

Analyzing the chip-specific hw_start functions all chip version from
34, with the exception of version 39, need this bit set.
This patch moves setting this bit to rtl_set_tx_config_registers().

Fixes: 4fd48c4ac0a0 ("r8169: move common initializations to tp->hw_start")
Reported-by: Ortwin Glück <odi@odi.ch>
Reported-by: David Arendt <admin@prnet.org>
Root-caused-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Tested-by: Tony Atkinson <tatkinson@linux.com>
Tested-by: David Arendt <admin@prnet.org>
Tested-by: Ortwin Glück <odi@odi.ch>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Hint for applying this change to stable:
It may collide with 05212ba8132b ("r8169: set RxConfig after tx/rx is
enabled for RTL8169sb/8110sb devices") which renamed
rtl_set_rx_tx_config_registers() to rtl_set_tx_config_registers().
---
v2:
- added Maciej as root-caused-by
---
 drivers/net/ethernet/realtek/r8169.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index f882be49f..ae8abe900 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4514,9 +4514,14 @@ static void rtl8169_hw_reset(struct rtl8169_private *tp)
 
 static void rtl_set_tx_config_registers(struct rtl8169_private *tp)
 {
-	/* Set DMA burst size and Interframe Gap Time */
-	RTL_W32(tp, TxConfig, (TX_DMA_BURST << TxDMAShift) |
-		(InterFrameGap << TxInterFrameGapShift));
+	u32 val = TX_DMA_BURST << TxDMAShift |
+		  InterFrameGap << TxInterFrameGapShift;
+
+	if (tp->mac_version >= RTL_GIGA_MAC_VER_34 &&
+	    tp->mac_version != RTL_GIGA_MAC_VER_39)
+		val |= TXCFG_AUTO_FIFO;
+
+	RTL_W32(tp, TxConfig, val);
 }
 
 static void rtl_set_rx_max_size(struct rtl8169_private *tp)
@@ -5011,7 +5016,6 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
 
 	rtl_disable_clock_request(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
 	/* Adjust EEE LED frequency */
@@ -5045,7 +5049,6 @@ static void rtl_hw_start_8168f(struct rtl8169_private *tp)
 
 	rtl_disable_clock_request(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
@@ -5090,8 +5093,6 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
 
 static void rtl_hw_start_8168g(struct rtl8169_private *tp)
 {
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
@@ -5189,8 +5190,6 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 	rtl_hw_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
@@ -5273,8 +5272,6 @@ static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
 {
 	rtl8168ep_stop_cmac(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x2f, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x5f, ERIAR_EXGMAC);
@@ -5596,7 +5593,6 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
 	/* Force LAN exit from ASPM if Rx/Tx are not idle */
 	RTL_W32(tp, FuncEvent, RTL_R32(tp, FuncEvent) | 0x002800);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
 	rtl_ephy_init(tp, e_info_8402, ARRAY_SIZE(e_info_8402));
-- 
2.19.0

^ permalink raw reply related

* [PATCH net 3/3] tun: napi flags belong to tfile
From: Eric Dumazet @ 2018-09-28 21:51 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180928215149.22092-1-edumazet@google.com>

Since tun->flags might be shared by multiple tfile structures,
it is better to make sure tun_get_user() is using the flags
for the current tfile.

Presence of the READ_ONCE() in tun_napi_frags_enabled() gave a hint
of what could happen, but we need something stronger to please
syzbot.

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 13647 Comm: syz-executor5 Not tainted 4.19.0-rc5+ #59
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:dev_gro_receive+0x132/0x2720 net/core/dev.c:5427
Code: 48 c1 ea 03 80 3c 02 00 0f 85 6e 20 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b 6e 10 49 8d bd d0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 59 20 00 00 4d 8b a5 d0 00 00 00 31 ff 41 81 e4
RSP: 0018:ffff8801c400f410 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8618d325
RDX: 000000000000001a RSI: ffffffff86189f97 RDI: 00000000000000d0
RBP: ffff8801c400f608 R08: ffff8801c8fb4300 R09: 0000000000000000
R10: ffffed0038801ed7 R11: 0000000000000003 R12: ffff8801d327d358
R13: 0000000000000000 R14: ffff8801c16dd8c0 R15: 0000000000000004
FS:  00007fe003615700(0000) GS:ffff8801dac00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe1f3c43db8 CR3: 00000001bebb2000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 napi_gro_frags+0x3f4/0xc90 net/core/dev.c:5715
 tun_get_user+0x31d5/0x42a0 drivers/net/tun.c:1922
 tun_chr_write_iter+0xb9/0x154 drivers/net/tun.c:1967
 call_write_iter include/linux/fs.h:1808 [inline]
 new_sync_write fs/read_write.c:474 [inline]
 __vfs_write+0x6b8/0x9f0 fs/read_write.c:487
 vfs_write+0x1fc/0x560 fs/read_write.c:549
 ksys_write+0x101/0x260 fs/read_write.c:598
 __do_sys_write fs/read_write.c:610 [inline]
 __se_sys_write fs/read_write.c:607 [inline]
 __x64_sys_write+0x73/0xb0 fs/read_write.c:607
 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457579
Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe003614c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457579
RDX: 0000000000000012 RSI: 0000000020000000 RDI: 000000000000000a
RBP: 000000000072c040 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fe0036156d4
R13: 00000000004c5574 R14: 00000000004d8e98 R15: 00000000ffffffff
Modules linked in:

RIP: 0010:dev_gro_receive+0x132/0x2720 net/core/dev.c:5427
Code: 48 c1 ea 03 80 3c 02 00 0f 85 6e 20 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b 6e 10 49 8d bd d0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 59 20 00 00 4d 8b a5 d0 00 00 00 31 ff 41 81 e4
RSP: 0018:ffff8801c400f410 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8618d325
RDX: 000000000000001a RSI: ffffffff86189f97 RDI: 00000000000000d0
RBP: ffff8801c400f608 R08: ffff8801c8fb4300 R09: 0000000000000000
R10: ffffed0038801ed7 R11: 0000000000000003 R12: ffff8801d327d358
R13: 0000000000000000 R14: ffff8801c16dd8c0 R15: 0000000000000004
FS:  00007fe003615700(0000) GS:ffff8801dac00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe1f3c43db8 CR3: 00000001bebb2000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 drivers/net/tun.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 729686babbf3b7d2f76ce64a5ebf7676e45eb681..50e9cc19023a701bad861ac117665a024ba776b1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -181,6 +181,7 @@ struct tun_file {
 	};
 	struct napi_struct napi;
 	bool napi_enabled;
+	bool napi_frags_enabled;
 	struct mutex napi_mutex;	/* Protects access to the above napi */
 	struct list_head next;
 	struct tun_struct *detached;
@@ -313,9 +314,10 @@ static int tun_napi_poll(struct napi_struct *napi, int budget)
 }
 
 static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile,
-			  bool napi_en)
+			  bool napi_en, bool napi_frags)
 {
 	tfile->napi_enabled = napi_en;
+	tfile->napi_frags_enabled = napi_en && napi_frags;
 	if (napi_en) {
 		netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
 			       NAPI_POLL_WEIGHT);
@@ -335,9 +337,9 @@ static void tun_napi_del(struct tun_file *tfile)
 		netif_napi_del(&tfile->napi);
 }
 
-static bool tun_napi_frags_enabled(const struct tun_struct *tun)
+static bool tun_napi_frags_enabled(const struct tun_file *tfile)
 {
-	return READ_ONCE(tun->flags) & IFF_NAPI_FRAGS;
+	return tfile->napi_frags_enabled;
 }
 
 #ifdef CONFIG_TUN_VNET_CROSS_LE
@@ -792,7 +794,7 @@ static void tun_detach_all(struct net_device *dev)
 }
 
 static int tun_attach(struct tun_struct *tun, struct file *file,
-		      bool skip_filter, bool napi)
+		      bool skip_filter, bool napi, bool napi_frags)
 {
 	struct tun_file *tfile = file->private_data;
 	struct net_device *dev = tun->dev;
@@ -865,7 +867,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 		tun_enable_queue(tfile);
 	} else {
 		sock_hold(&tfile->sk);
-		tun_napi_init(tun, tfile, napi);
+		tun_napi_init(tun, tfile, napi, napi_frags);
 	}
 
 	tun_set_real_num_queues(tun);
@@ -1708,7 +1710,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	int err;
 	u32 rxhash = 0;
 	int skb_xdp = 1;
-	bool frags = tun_napi_frags_enabled(tun);
+	bool frags = tun_napi_frags_enabled(tfile);
 
 	if (!(tun->dev->flags & IFF_UP))
 		return -EIO;
@@ -2533,7 +2535,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			return err;
 
 		err = tun_attach(tun, file, ifr->ifr_flags & IFF_NOFILTER,
-				 ifr->ifr_flags & IFF_NAPI);
+				 ifr->ifr_flags & IFF_NAPI,
+				 ifr->ifr_flags & IFF_NAPI_FRAGS);
 		if (err < 0)
 			return err;
 
@@ -2631,7 +2634,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			      (ifr->ifr_flags & TUN_FEATURES);
 
 		INIT_LIST_HEAD(&tun->disabled);
-		err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI);
+		err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI,
+				 ifr->ifr_flags & IFF_NAPI_FRAGS);
 		if (err < 0)
 			goto err_free_flow;
 
@@ -2780,7 +2784,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 		ret = security_tun_dev_attach_queue(tun->security);
 		if (ret < 0)
 			goto unlock;
-		ret = tun_attach(tun, file, false, tun->flags & IFF_NAPI);
+		ret = tun_attach(tun, file, false, tun->flags & IFF_NAPI,
+				 tun->flags & IFF_NAPI_FRAGS);
 	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
 		tun = rtnl_dereference(tfile->tun);
 		if (!tun || !(tun->flags & IFF_MULTI_QUEUE) || tfile->detached)
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 2/3] tun: initialize napi_mutex unconditionally
From: Eric Dumazet @ 2018-09-28 21:51 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180928215149.22092-1-edumazet@google.com>

This is the first part to fix following syzbot report :

console output: https://syzkaller.appspot.com/x/log.txt?x=145378e6400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=443816db871edd66
dashboard link: https://syzkaller.appspot.com/bug?extid=e662df0ac1d753b57e80

Following patch is fixing the race condition, but it seems safer
to initialize this mutex at tfile creation anyway.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot+e662df0ac1d753b57e80@syzkaller.appspotmail.com
---
 drivers/net/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 71d10fb59849bff091ee64b6f7e9cc8ae2e0cf6f..729686babbf3b7d2f76ce64a5ebf7676e45eb681 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -320,7 +320,6 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile,
 		netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
 			       NAPI_POLL_WEIGHT);
 		napi_enable(&tfile->napi);
-		mutex_init(&tfile->napi_mutex);
 	}
 }
 
@@ -3199,6 +3198,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 		return -ENOMEM;
 	}
 
+	mutex_init(&tfile->napi_mutex);
 	RCU_INIT_POINTER(tfile->tun, NULL);
 	tfile->flags = 0;
 	tfile->ifindex = 0;
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 1/3] tun: remove unused parameters
From: Eric Dumazet @ 2018-09-28 21:51 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180928215149.22092-1-edumazet@google.com>

tun_napi_disable() and tun_napi_del() do not need
a pointer to the tun_struct

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/tun.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e2648b5a3861e51dc6c40d19e1198a5f3f7ca7af..71d10fb59849bff091ee64b6f7e9cc8ae2e0cf6f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -324,13 +324,13 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile,
 	}
 }
 
-static void tun_napi_disable(struct tun_struct *tun, struct tun_file *tfile)
+static void tun_napi_disable(struct tun_file *tfile)
 {
 	if (tfile->napi_enabled)
 		napi_disable(&tfile->napi);
 }
 
-static void tun_napi_del(struct tun_struct *tun, struct tun_file *tfile)
+static void tun_napi_del(struct tun_file *tfile)
 {
 	if (tfile->napi_enabled)
 		netif_napi_del(&tfile->napi);
@@ -690,8 +690,8 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 	tun = rtnl_dereference(tfile->tun);
 
 	if (tun && clean) {
-		tun_napi_disable(tun, tfile);
-		tun_napi_del(tun, tfile);
+		tun_napi_disable(tfile);
+		tun_napi_del(tfile);
 	}
 
 	if (tun && !tfile->detached) {
@@ -758,7 +758,7 @@ static void tun_detach_all(struct net_device *dev)
 	for (i = 0; i < n; i++) {
 		tfile = rtnl_dereference(tun->tfiles[i]);
 		BUG_ON(!tfile);
-		tun_napi_disable(tun, tfile);
+		tun_napi_disable(tfile);
 		tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
 		tfile->socket.sk->sk_data_ready(tfile->socket.sk);
 		RCU_INIT_POINTER(tfile->tun, NULL);
@@ -774,7 +774,7 @@ static void tun_detach_all(struct net_device *dev)
 	synchronize_net();
 	for (i = 0; i < n; i++) {
 		tfile = rtnl_dereference(tun->tfiles[i]);
-		tun_napi_del(tun, tfile);
+		tun_napi_del(tfile);
 		/* Drop read queue */
 		tun_queue_purge(tfile);
 		xdp_rxq_info_unreg(&tfile->xdp_rxq);
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 0/3] tun: address two syzbot reports
From: Eric Dumazet @ 2018-09-28 21:51 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Small changes addressing races discovered by syzbot.

First patch is a cleanup.
Second patch moves a mutex init sooner.
Third patch makes sure each tfile gets its own napi enable flags.

Eric Dumazet (3):
  tun: remove unused parameters
  tun: initialize napi_mutex unconditionally
  tun: napi flags belong to tfile

 drivers/net/tun.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply

* Re: [net] r8169: fix network stalls due to missing bit TXCFG_AUTO_FIFO
From: Maciej S. Szmigiero @ 2018-09-28 21:47 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org, Tony Atkinson, David Arendt,
	Ortwin Glück
In-Reply-To: <141198ba-36e5-ba5c-e96a-8d4c08f810db@gmail.com>

On 28.09.2018 22:19, Heiner Kallweit wrote:
> Some of the chip-specific hw_start functions set bit TXCFG_AUTO_FIFO
> in register TxConfig. The original patch changed the order of some
> calls resulting in these changes being overwritten by
> rtl_set_tx_config_registers() in rtl_hw_start(). This eventually
> resulted in network stalls especially under high load.
> 
> Analyzing the chip-specific hw_start functions all chip version from
> 34, with the exception of version 39, need this bit set.
> This patch moves setting this bit to rtl_set_tx_config_registers().
> 
> Fixes: 4fd48c4ac0a0 ("r8169: move common initializations to tp->hw_start")
> Reported-by: Ortwin Glück <odi@odi.ch>
> Reported-by: David Arendt <admin@prnet.org>
> Tested-by: Tony Atkinson <tatkinson@linux.com>
> Tested-by: David Arendt <admin@prnet.org>
> Tested-by: Ortwin Glück <odi@odi.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Please add:
Root-caused-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>

Thanks,
Maciej

^ permalink raw reply

* Re: [PATCH net-next] tcp/fq: move back to CLOCK_MONOTONIC
From: Leonard Crestez @ 2018-09-28 21:27 UTC (permalink / raw)
  To: davem@davemloft.net, edumazet@google.com
  Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <20180928172844.182542-1-edumazet@google.com>

On Fri, 2018-09-28 at 10:28 -0700, Eric Dumazet wrote:
> In the recent TCP/EDT patch series, I switched TCP and sch_fq
> clocks from MONOTONIC to TAI, in order to meet the choice done
> earlier for sch_etf packet scheduler.
> 
> But sure enough, this broke some setups were the TAI clock
> jumps forward (by almost 50 year...), as reported
> by Leonard Crestez.
> 
> If we want to converge later, we'll probably need to add
> an skb field to differentiate the clock bases, or a socket option.
> 
> In the meantime, an UDP application will need to use CLOCK_MONOTONIC
> base for its SCM_TXTIME timestamps if using fq packet scheduler.
> 
> Fixes: 72b0094f9182 ("tcp: switch tcp_clock_ns() to CLOCK_TAI base")
> Fixes: 142537e41923 ("net_sched: sch_fq: switch to CLOCK_TAI")
> Fixes: fd2bca2aa789 ("tcp: switch internal pacing timer to CLOCK_TAI")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>

Tested-by: Leonard Crestez <leonard.crestez@nxp.com>

Fixes the problem reported earlier when applied on top of next-20180928

^ permalink raw reply

* RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
From: Y.b. Lu @ 2018-09-29  3:19 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: devel@driverdev.osuosl.org, netdev@vger.kernel.org,
	Richard Cochran, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	David S . Miller
In-Reply-To: <20180928151750.GD19396@lunn.ch>

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, September 28, 2018 11:18 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> David S . Miller <davem@davemloft.net>; Ioana Ciocoi Radulescu
> <ruxandra.radulescu@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> > > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
> > > seems very odd. And it is not the only example.
> >
> > [Y.b. Lu] This should depended on MC firmware and APIs I think. Once the
> MC improves this, the APIs could be updated to fix this.
> 
> That is going to be hard to do. Ideally the driver should work with any
> firmware version. You don't really want to force the user to upgrade the
> driver/kernel and the firmware at the same time. So you cannot for example
> remove this pad. What you might be able to do in newer versions is actually
> use the space. But you have to be sure the current code is correctly ignoring it
> and setting it to zero.

[Y.b. Lu] Thanks a lot, I think I understand now😊
The files dprtc* defining the APIs were provided together with MC firmware. They were tested working fine.
MC firmware would also consider the backward compatibility I think.
Regarding to the API files, let me remove unused code before using them, and keep the rest.

> 
> 	Andrew
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
From: Y.b. Lu @ 2018-09-29  3:06 UTC (permalink / raw)
  To: Ioana Ciocoi Radulescu, Andrew Lunn
  Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	netdev@vger.kernel.org, Richard Cochran, David S . Miller,
	Greg Kroah-Hartman
In-Reply-To: <DBXPR04MB3490DB9A1EF2D814568843B94EC0@DBXPR04MB349.eurprd04.prod.outlook.com>

Hi Ioana,

> -----Original Message-----
> From: Ioana Ciocoi Radulescu
> Sent: Friday, September 28, 2018 6:21 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> David S . Miller <davem@davemloft.net>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> > -----Original Message-----
> > From: Y.b. Lu
> > Sent: Friday, September 28, 2018 11:04 AM
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> > netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> > David S . Miller <davem@davemloft.net>; Ioana Ciocoi Radulescu
> > <ruxandra.radulescu@nxp.com>; Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org>
> > Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of
> > staging/
> >
[...]
> > >
> > > It seems like there is a lot of code in dprtc.c which is unused.
> > > rtc.c does
> > nothing
> > > with interrupts for example. Do you plan to make use of this extra
> > > code? Or can it be removed leaving just what is needed?
> >
> > [Y.b. Lu] Currently the ptp/rtc driver is not full-featured. The extra
> > code is being planed to be used.
> 
> Are there any interrupts associated to the real time clock module that will
> actually be used by the driver? Also, I don't think the create/destroy functions
> are meant to be used by the PTP kernel driver, even though MC exposes the
> APIs for them.
> 
> Generally speaking, I think it's better to remove unused code from the current
> driver and re-add it along with the feature actually using it.

[Y.b. Lu] Yes. We need to implement these interrupts to support ptp_clock_event() of common ptp_clock driver.
This is mainly to support 1588 timer external signals.
I get your point, and will remove unused code before using them.

> 
> >
> > >
> > > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
> > > seems
> > very
> > > odd. And it is not the only example.
> >
> > [Y.b. Lu] This should depended on MC firmware and APIs I think. Once
> > the MC improves this, the APIs could be updated to fix this.
> 
> These structures map the command format expected by the MC firmware. I
> agree that some of the command layouts are less than inspired, but I'm not
> sure we can expect MC to "improve" them without a good reason, as this
> would break backward compatibility.
> 
> I also want to bring up the question of where the dpaa2 ptp driver should be
> located. The qoriq_ptp driver (which targets previous gen Freescale/NXP
> architectures) is located in drivers/ptp. I'm not sure if the dpaa2 ptp driver
> should be moved there as well or it's better suited for the currently proposed
> location.

[Y.b. Lu] Actually the ptp timer is to provide hw timestamping support for ethernet.
Ptp clock driver together with ethernet hw timestamping driver provide the method to support 1588 software application.
It's ok to put ptp clock driver near to ethernet driver. And most ptp clock drivers in kernel are together with ethernet driver.
You may see there are gianfar_ptp and dpaa_ptp before. Considering they could reuse the code, I created the ptp_qoriq for them to use the one driver.

> 
> Thanks,
> Ioana

^ permalink raw reply

* bond: take rcu lock in netpoll_send_skb_on_dev
From: Dave Jones @ 2018-09-28 20:26 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang
In-Reply-To: <20180924192317.wghbs34ld7klqpi6@codemonkey.org.uk>

The bonding driver lacks the rcu lock when it calls down into
netdev_lower_get_next_private_rcu from bond_poll_controller, which
results in a trace like:

WARNING: CPU: 2 PID: 179 at net/core/dev.c:6567 netdev_lower_get_next_private_rcu+0x34/0x40
CPU: 2 PID: 179 Comm: kworker/u16:15 Not tainted 4.19.0-rc5-backup+ #1
Workqueue: bond0 bond_mii_monitor
RIP: 0010:netdev_lower_get_next_private_rcu+0x34/0x40
Code: 48 89 fb e8 fe 29 63 ff 85 c0 74 1e 48 8b 45 00 48 81 c3 c0 00 00 00 48 8b 00 48 39 d8 74 0f 48 89 45 00 48 8b 40 f8 5b 5d c3 <0f> 0b eb de 31 c0 eb f5 0f 1f 40 00 0f 1f 44 00 00 48 8>
RSP: 0018:ffffc9000087fa68 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff880429614560 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 00000000ffffffff RDI: ffffffffa184ada0
RBP: ffffc9000087fa80 R08: 0000000000000001 R09: 0000000000000000
R10: ffffc9000087f9f0 R11: ffff880429798040 R12: ffff8804289d5980
R13: ffffffffa1511f60 R14: 00000000000000c8 R15: 00000000ffffffff
FS:  0000000000000000(0000) GS:ffff88042f880000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4b78fce180 CR3: 000000018180f006 CR4: 00000000001606e0
Call Trace:
 bond_poll_controller+0x52/0x170
 netpoll_poll_dev+0x79/0x290
 netpoll_send_skb_on_dev+0x158/0x2c0
 netpoll_send_udp+0x2d5/0x430
 write_ext_msg+0x1e0/0x210
 console_unlock+0x3c4/0x630
 vprintk_emit+0xfa/0x2f0
 printk+0x52/0x6e
 ? __netdev_printk+0x12b/0x220
 netdev_info+0x64/0x80
 ? bond_3ad_set_carrier+0xe9/0x180
 bond_select_active_slave+0x1fc/0x310
 bond_mii_monitor+0x709/0x9b0
 process_one_work+0x221/0x5e0
 worker_thread+0x4f/0x3b0
 kthread+0x100/0x140
 ? process_one_work+0x5e0/0x5e0
 ? kthread_delayed_work_timer_fn+0x90/0x90
 ret_from_fork+0x24/0x30

We're also doing rcu dereferences a layer up in netpoll_send_skb_on_dev
before we call down into netpoll_poll_dev, so just take the lock there.

Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Dave Jones <davej@codemonkey.org.uk>

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 3219a2932463..692367d7c280 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -330,6 +330,7 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
 	/* It is up to the caller to keep npinfo alive. */
 	struct netpoll_info *npinfo;
 
+	rcu_read_lock_bh();
 	lockdep_assert_irqs_disabled();
 
 	npinfo = rcu_dereference_bh(np->dev->npinfo);
@@ -374,6 +375,7 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
 		skb_queue_tail(&npinfo->txq, skb);
 		schedule_delayed_work(&npinfo->tx_work,0);
 	}
+	rcu_read_unlock_bh();
 }
 EXPORT_SYMBOL(netpoll_send_skb_on_dev);
 

^ permalink raw reply related

* [PATCH net] r8169: fix network stalls due to missing bit TXCFG_AUTO_FIFO
From: Heiner Kallweit @ 2018-09-28 20:19 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Tony Atkinson, David Arendt,
	Ortwin Glück

Some of the chip-specific hw_start functions set bit TXCFG_AUTO_FIFO
in register TxConfig. The original patch changed the order of some
calls resulting in these changes being overwritten by
rtl_set_tx_config_registers() in rtl_hw_start(). This eventually
resulted in network stalls especially under high load.

Analyzing the chip-specific hw_start functions all chip version from
34, with the exception of version 39, need this bit set.
This patch moves setting this bit to rtl_set_tx_config_registers().

Fixes: 4fd48c4ac0a0 ("r8169: move common initializations to tp->hw_start")
Reported-by: Ortwin Glück <odi@odi.ch>
Reported-by: David Arendt <admin@prnet.org>
Tested-by: Tony Atkinson <tatkinson@linux.com>
Tested-by: David Arendt <admin@prnet.org>
Tested-by: Ortwin Glück <odi@odi.ch>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Hint for applying this change to stable:
It may collide with 05212ba8132b ("r8169: set RxConfig after tx/rx is
enabled for RTL8169sb/8110sb devices") which renamed
rtl_set_rx_tx_config_registers() to rtl_set_tx_config_registers().
---
 drivers/net/ethernet/realtek/r8169.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index f882be49f..ae8abe900 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4514,9 +4514,14 @@ static void rtl8169_hw_reset(struct rtl8169_private *tp)
 
 static void rtl_set_tx_config_registers(struct rtl8169_private *tp)
 {
-	/* Set DMA burst size and Interframe Gap Time */
-	RTL_W32(tp, TxConfig, (TX_DMA_BURST << TxDMAShift) |
-		(InterFrameGap << TxInterFrameGapShift));
+	u32 val = TX_DMA_BURST << TxDMAShift |
+		  InterFrameGap << TxInterFrameGapShift;
+
+	if (tp->mac_version >= RTL_GIGA_MAC_VER_34 &&
+	    tp->mac_version != RTL_GIGA_MAC_VER_39)
+		val |= TXCFG_AUTO_FIFO;
+
+	RTL_W32(tp, TxConfig, val);
 }
 
 static void rtl_set_rx_max_size(struct rtl8169_private *tp)
@@ -5011,7 +5016,6 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
 
 	rtl_disable_clock_request(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
 	/* Adjust EEE LED frequency */
@@ -5045,7 +5049,6 @@ static void rtl_hw_start_8168f(struct rtl8169_private *tp)
 
 	rtl_disable_clock_request(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
@@ -5090,8 +5093,6 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
 
 static void rtl_hw_start_8168g(struct rtl8169_private *tp)
 {
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
@@ -5189,8 +5190,6 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 	rtl_hw_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
@@ -5273,8 +5272,6 @@ static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
 {
 	rtl8168ep_stop_cmac(tp);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
-
 	rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x2f, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x5f, ERIAR_EXGMAC);
@@ -5596,7 +5593,6 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
 	/* Force LAN exit from ASPM if Rx/Tx are not idle */
 	RTL_W32(tp, FuncEvent, RTL_R32(tp, FuncEvent) | 0x002800);
 
-	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
 	rtl_ephy_init(tp, e_info_8402, ARRAY_SIZE(e_info_8402));
-- 
2.19.0

^ permalink raw reply related

* [PATCH net-next v2] tcp: up initial rmem to 128KB and SYN rwin to around 64KB
From: Yuchung Cheng @ 2018-09-28 20:09 UTC (permalink / raw)
  To: davem, edumazet; +Cc: netdev, ncardwell, weiwan, soheil, Yuchung Cheng

Previously TCP initial receive buffer is ~87KB by default and
the initial receive window is ~29KB (20 MSS). This patch changes
the two numbers to 128KB and ~64KB (rounding down to the multiples
of MSS) respectively. The patch also simplifies the calculations s.t.
the two numbers are directly controlled by sysctl tcp_rmem[1]:

  1) Initial receiver buffer budget (sk_rcvbuf): while this should
     be configured via sysctl tcp_rmem[1], previously tcp_fixup_rcvbuf()
     always override and set a larger size when a new connection
     establishes.

  2) Initial receive window in SYN: previously it is set to 20
     packets if MSS <= 1460. The number 20 was based on the initial
     congestion window of 10: the receiver needs twice amount to
     avoid being limited by the receive window upon out-of-order
     delivery in the first window burst. But since this only
     applies if the receiving MSS <= 1460, connection using large MTU
     (e.g. to utilize receiver zero-copy) may be limited by the
     receive window.

This patch also lowers the initial bytes expected to receive in
the receiver buffer autotuning algorithm - otherwise the receiver
may take two to three rounds to increase the buffer to the
appropriate level (2x sender congestion window).

With this patch TCP memory configuration is more straight-forward and
more properly sized to modern high-speed networks by default. Several
popular stacks have been announcing 64KB rwin in SYNs as well.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Soheil Hassas Yeganeh <soheil@google.com>
---
 net/ipv4/tcp.c        |  4 ++--
 net/ipv4/tcp_input.c  | 30 +++++-------------------------
 net/ipv4/tcp_output.c | 25 ++++---------------------
 3 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 69c236943f56..dcf51fbf5ec7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3896,8 +3896,8 @@ void __init tcp_init(void)
 	init_net.ipv4.sysctl_tcp_wmem[2] = max(64*1024, max_wshare);
 
 	init_net.ipv4.sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
-	init_net.ipv4.sysctl_tcp_rmem[1] = 87380;
-	init_net.ipv4.sysctl_tcp_rmem[2] = max(87380, max_rshare);
+	init_net.ipv4.sysctl_tcp_rmem[1] = 131072;
+	init_net.ipv4.sysctl_tcp_rmem[2] = max(131072, max_rshare);
 
 	pr_info("Hash tables configured (established %u bind %u)\n",
 		tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d703a0b3b6a2..4f714a031618 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -426,27 +426,9 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
 	}
 }
 
-/* 3. Tuning rcvbuf, when connection enters established state. */
-static void tcp_fixup_rcvbuf(struct sock *sk)
-{
-	u32 mss = tcp_sk(sk)->advmss;
-	int rcvmem;
-
-	rcvmem = 2 * SKB_TRUESIZE(mss + MAX_TCP_HEADER) *
-		 tcp_default_init_rwnd(mss);
-
-	/* Dynamic Right Sizing (DRS) has 2 to 3 RTT latency
-	 * Allow enough cushion so that sender is not limited by our window
-	 */
-	if (sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf)
-		rcvmem <<= 2;
-
-	if (sk->sk_rcvbuf < rcvmem)
-		sk->sk_rcvbuf = min(rcvmem, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
-}
-
-/* 4. Try to fixup all. It is made immediately after connection enters
- *    established state.
+/* 3. Try to fixup all. It is made immediately after connection enters
+ *    established state. Budget the space to the expected initial window
+ *    of burst to auto-tune the receive buffer right after the first round.
  */
 void tcp_init_buffer_space(struct sock *sk)
 {
@@ -454,12 +436,10 @@ void tcp_init_buffer_space(struct sock *sk)
 	struct tcp_sock *tp = tcp_sk(sk);
 	int maxwin;
 
-	if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK))
-		tcp_fixup_rcvbuf(sk);
 	if (!(sk->sk_userlocks & SOCK_SNDBUF_LOCK))
 		tcp_sndbuf_expand(sk);
 
-	tp->rcvq_space.space = tp->rcv_wnd;
+	tp->rcvq_space.space = min_t(u32, tp->rcv_wnd, TCP_INIT_CWND * tp->advmss);
 	tcp_mstamp_refresh(tp);
 	tp->rcvq_space.time = tp->tcp_mstamp;
 	tp->rcvq_space.seq = tp->copied_seq;
@@ -485,7 +465,7 @@ void tcp_init_buffer_space(struct sock *sk)
 	tp->snd_cwnd_stamp = tcp_jiffies32;
 }
 
-/* 5. Recalculate window clamp after socket hit its memory bounds. */
+/* 4. Recalculate window clamp after socket hit its memory bounds. */
 static void tcp_clamp_window(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index fe7855b090e4..059b67af28b1 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -195,21 +195,6 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts,
 	inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
 }
 
-
-u32 tcp_default_init_rwnd(u32 mss)
-{
-	/* Initial receive window should be twice of TCP_INIT_CWND to
-	 * enable proper sending of new unsent data during fast recovery
-	 * (RFC 3517, Section 4, NextSeg() rule (2)). Further place a
-	 * limit when mss is larger than 1460.
-	 */
-	u32 init_rwnd = TCP_INIT_CWND * 2;
-
-	if (mss > 1460)
-		init_rwnd = max((1460 * init_rwnd) / mss, 2U);
-	return init_rwnd;
-}
-
 /* Determine a window scaling and initial window to offer.
  * Based on the assumption that the given amount of space
  * will be offered. Store the results in the tp structure.
@@ -244,7 +229,10 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
 	if (sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows)
 		(*rcv_wnd) = min(space, MAX_TCP_WINDOW);
 	else
-		(*rcv_wnd) = space;
+		(*rcv_wnd) = min_t(u32, space, U16_MAX);
+
+	if (init_rcv_wnd)
+		*rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
 
 	(*rcv_wscale) = 0;
 	if (wscale_ok) {
@@ -257,11 +245,6 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
 			(*rcv_wscale)++;
 		}
 	}
-
-	if (!init_rcv_wnd) /* Use default unless specified otherwise */
-		init_rcv_wnd = tcp_default_init_rwnd(mss);
-	*rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
-
 	/* Set the clamp no higher than max representable value */
 	(*window_clamp) = min_t(__u32, U16_MAX << (*rcv_wscale), *window_clamp);
 }
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-29  2:20 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
	Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
	Jean-Philippe Aumasson, Russell King - ARM Linux,
	linux-arm-kernel
In-Reply-To: <CAKv+Gu8knzzmK4_KouFQWKzMdHFgEsk-CtBrcuAvk0bYkDFO=w@mail.gmail.com>

Hi Ard,

On Fri, Sep 28, 2018 at 6:02 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Please put comments like this below the ---

git-notes is nice for this indeed.

> Are these CONFIG_ symbols defined anywhere at this point?

Yes, they're introduced in the first zinc commit. There's no git-blame
on git.kernel.org, presumably because it's expensive to compute, but
there is on my personal instance, so this might help:
https://git.zx2c4.com/linux-dev/blame/lib/zinc/Kconfig?h=jd/wireguard

> In any case, I don't think these is a reason for these, at least not
> on ARM/arm64. The 64-bitness is implied in both cases

You mean to say that since these nobs are def_bool y and are
essentially "depends on ARM", then I should just straight up use
CONFIG_ARM? I had thought about this, but figured this would make it
easier to later make these optional or have other options block them
need be, or even if the dependencies and requirements for having them
changes (for example, with UML on x86). I think doing it this way
gives us some flexibility later on. So if that's a compelling enough
reason, I'd like to keep those.

> and the
> dependency on !CPU_32v3 you introduce (looking at the version of
> Kconfig at the end of the series) seems spurious to me. Was that added
> because of some kbuild robot report? (we don't support ARMv3 in the
> kernel but ARCH_RPC is built in v3 mode because of historical reasons
> while the actual core is a v4)

I added the !CPU_32v3 in my development tree after posting v6, so good
to hear you're just looking straight at the updated tree. If you see
things jump out in there prior to me posting v7, don't hesitate to let
me know.

The reason it was added was indeed because of:
https://lists.01.org/pipermail/kbuild-all/2018-September/053114.html
-- exactly what you suspected, ARCH_RPC. Have a better suggestion than
!CPU_32v3? It seems to me like so long as the kernel has CPU_32v3 as a
thing in any form, I should mark Zinc as not supporting it, since
we'll certainly be at least v4 and up. (Do you guys have any old Acorn
ARM610 boxes sitting around for old time's sake at LinaroHQ? ;-)

> > +#endif
> > +
>
> No need to make asmlinkage declarations conditional

Yep, addressed in the IS_ENABLED cleanup.

>
> if (IS_ENABLED())

Sorted.

>
> """
> if (!IS_ENABLED(CONFIG_ARM))
>    return false;
>
> hchacha20_arm(x, derived_key);
> return true;
> """
>
> and drop the #ifdefs

Also sorted.

Regards,
Jason

^ 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