Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports
From: Jakub Kicinski @ 2019-02-26 18:24 UTC (permalink / raw)
  To: davem, jiri; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20190226182436.23811-1-jakub.kicinski@netronome.com>

PCI endpoint corresponds to a PCI device, but such device
can have one more more logical device ports associated with it.
We need a way to distinguish those. Add a PCI subport in the
dumps and print the info in phys_port_name appropriately.

This is not equivalent to port splitting, there is no split
group. It's just a way of representing multiple netdevs on
a single PCI function.

Note that the quality of being multiport pertains only to
the PCI function itself. A PF having multiple netdevs does
not mean that its VFs will also have multiple, or that VFs
are associated with any particular port of a multiport VF.

Example (bus 05 device has subports, bus 82 has only one port per
function):

$ devlink port
pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical
pci/0000:05:00.0/10000: type eth netdev enp5s0npf0s0 flavour pci_pf pf 0 subport 0
pci/0000:05:00.0/4: type eth netdev enp5s0np1 flavour physical
pci/0000:05:00.0/11000: type eth netdev enp5s0npf0s1 flavour pci_pf pf 0 subport 1
pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
pci/0000:82:00.0/10000: type eth netdev eth0 flavour pci_pf pf 0

$ devlink -jp port
{
    "port": {
        "pci/0000:05:00.0/0": {
            "type": "eth",
            "netdev": "enp5s0np0",
            "flavour": "physical"
        },
        "pci/0000:05:00.0/10000": {
            "type": "eth",
            "netdev": "enp5s0npf0s0",
            "flavour": "pci_pf",
            "pf": 0,
            "subport": 0
        },
        "pci/0000:05:00.0/4": {
            "type": "eth",
            "netdev": "enp5s0np1",
            "flavour": "physical"
        },
        "pci/0000:05:00.0/11000": {
            "type": "eth",
            "netdev": "enp5s0npf0s1",
            "flavour": "pci_pf",
            "pf": 0,
            "subport": 1
        },
        "pci/0000:82:00.0/0": {
            "type": "eth",
            "netdev": "p4p1",
            "flavour": "physical"
        },
        "pci/0000:82:00.0/10000": {
            "type": "eth",
            "netdev": "eth0",
            "flavour": "pci_pf",
            "pf": 0
        }
    }
}

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  3 +-
 include/net/devlink.h                         |  9 ++++--
 include/uapi/linux/devlink.h                  |  1 +
 net/core/devlink.c                            | 29 ++++++++++++++++---
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 36976e37d162..bb07be4117a3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -375,7 +375,8 @@ nfp_devlink_port_init_pci(struct devlink *devlink, struct nfp_port *port,
 {
 	devlink_port_type_eth_set(&port->dl_port, port->netdev);
 	devlink_port_attrs_pci_set(&port->dl_port, flavour,
-				   port->pf_id, port->vf_id);
+				   port->pf_id, port->vf_id,
+				   port->pf_split, port->pf_split_id);
 	return 0;
 }
 
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b5376ef492f1..13e0a479c546 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -53,6 +53,8 @@ struct devlink_port_attrs {
 		struct {
 			u32 pf_number;
 			u32 vf_number;
+			bool multiport;
+			u32 subport_number;
 		} pci;
 	};
 };
@@ -580,7 +582,8 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    u32 split_subport_number);
 void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
 				enum devlink_port_flavour flavour,
-				u32 pf_number, u32 vf_number);
+				u32 pf_number, u32 vf_number,
+				bool multiport, u32 subport_number);
 int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
 				    char *name, size_t len);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
@@ -797,7 +800,9 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
 
 static inline void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
 					      enum devlink_port_flavour flavour,
-					      u32 pf_number, u32 vf_number)
+					      u32 pf_number, u32 vf_number,
+					      bool multiport,
+					      u32 subport_number)
 {
 }
 
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 9ce76d4f640d..417ae8233cce 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -336,6 +336,7 @@ enum devlink_attr {
 
 	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u32 */
 	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u32 */
+	DEVLINK_ATTR_PORT_PCI_SUBPORT,		/* u32 */
 
 	/* add new attributes above here, update the policy in devlink.c */
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index af177284830b..6cdf7e87d7fc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -541,6 +541,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
 				attrs->pci.pf_number))
 			return -EMSGSIZE;
+
+		if (attrs->pci.multiport &&
+		    nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_SUBPORT,
+				attrs->pci.subport_number))
+			return -EMSGSIZE;
 		return 0;
 	default:
 		return -EINVAL;
@@ -5449,10 +5454,13 @@ EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
  *	@pf_number: PCI PF number, in multi-host mapping to hosts depends
  *	            on the platform
  *	@vf_number: PCI VF number within given PF (ignored for PF itself)
+ *	@multiport: PCI function has more than one logical port
+ *	@subport_number: PCI function has more than one logical port
  */
 void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
 				enum devlink_port_flavour flavour,
-				u32 pf_number, u32 vf_number)
+				u32 pf_number, u32 vf_number,
+				bool multiport, u32 subport_number)
 {
 	struct devlink_port_attrs *attrs = &devlink_port->attrs;
 
@@ -5463,6 +5471,8 @@ void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
 	attrs->flavour = flavour;
 	attrs->pci.pf_number = pf_number;
 	attrs->pci.vf_number = vf_number;
+	attrs->pci.multiport = multiport;
+	attrs->pci.subport_number = subport_number;
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_set);
@@ -5492,11 +5502,22 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
 		WARN_ON(1);
 		return -EINVAL;
 	case DEVLINK_PORT_FLAVOUR_PCI_PF:
-		n = snprintf(name, len, "pf%u", attrs->pci.pf_number);
+		if (!attrs->pci.multiport)
+			n = snprintf(name, len, "pf%u", attrs->pci.pf_number);
+		else
+			n = snprintf(name, len, "pf%us%u", attrs->pci.pf_number,
+				     attrs->pci.subport_number);
 		break;
 	case DEVLINK_PORT_FLAVOUR_PCI_VF:
-		n = snprintf(name, len, "pf%uvf%u",
-			     attrs->pf_number, attrs->pci.vf_number);
+		if (!attrs->pci.multiport)
+			n = snprintf(name, len, "pf%uvf%u",
+				     attrs->pci.pf_number,
+				     attrs->pci.vf_number);
+		else
+			n = snprintf(name, len, "pf%uvf%us%u",
+				     attrs->pci.pf_number,
+				     attrs->pci.vf_number,
+				     attrs->pci.subport_number);
 		break;
 	}
 
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 1/8] nfp: split devlink port init from registration
From: Jakub Kicinski @ 2019-02-26 18:24 UTC (permalink / raw)
  To: davem, jiri; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20190226182436.23811-1-jakub.kicinski@netronome.com>

In the future we want to switch to using the
devlink_port_get_phys_port_name() helper for .ndo_phys_port_name.
This requires that we initialize the devlink ports before the
netdevs are registered. Split the registration from init.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/nfp_devlink.c    | 13 ++++++++++---
 .../net/ethernet/netronome/nfp/nfp_net_main.c   | 17 ++++++++++++++---
 drivers/net/ethernet/netronome/nfp/nfp_port.h   |  2 ++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index e9eca99cf493..9af3cb1f2f17 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -350,10 +350,9 @@ const struct devlink_ops nfp_devlink_ops = {
 	.flash_update		= nfp_devlink_flash_update,
 };
 
-int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
+int nfp_devlink_port_init(struct nfp_app *app, struct nfp_port *port)
 {
 	struct nfp_eth_table_port eth_port;
-	struct devlink *devlink;
 	int ret;
 
 	rtnl_lock();
@@ -366,8 +365,16 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       eth_port.label_port, eth_port.is_split,
 			       eth_port.label_subport);
+	return 0;
+}
+
+void nfp_devlink_port_clean(struct nfp_port *port)
+{
+}
 
-	devlink = priv_to_devlink(app->pf);
+int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
+{
+	struct devlink *devlink = priv_to_devlink(app->pf);
 
 	return devlink_port_register(devlink, &port->dl_port, port->eth_id);
 }
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 08f5fdbd8e41..39e87bb73ebf 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -150,9 +150,15 @@ nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
 
 	nn->id = id;
 
+	if (nn->port) {
+		err = nfp_devlink_port_init(pf->app, nn->port);
+		if (err)
+			return err;
+	}
+
 	err = nfp_net_init(nn);
 	if (err)
-		return err;
+		goto err_port_clean;
 
 	nfp_net_debugfs_vnic_add(nn, pf->ddir);
 
@@ -167,17 +173,20 @@ nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
 	if (nfp_net_is_data_vnic(nn)) {
 		err = nfp_app_vnic_init(pf->app, nn);
 		if (err)
-			goto err_devlink_port_clean;
+			goto err_port_unreg;
 	}
 
 	return 0;
 
-err_devlink_port_clean:
+err_port_unreg:
 	if (nn->port)
 		nfp_devlink_port_unregister(nn->port);
 err_dfs_clean:
 	nfp_net_debugfs_dir_clean(&nn->debugfs_dir);
 	nfp_net_clean(nn);
+err_port_clean:
+	if (nn->port)
+		nfp_devlink_port_clean(nn->port);
 	return err;
 }
 
@@ -224,6 +233,8 @@ static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
 		nfp_devlink_port_unregister(nn->port);
 	nfp_net_debugfs_dir_clean(&nn->debugfs_dir);
 	nfp_net_clean(nn);
+	if (nn->port)
+		nfp_devlink_port_clean(nn->port);
 }
 
 static int nfp_net_pf_alloc_irqs(struct nfp_pf *pf)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.h b/drivers/net/ethernet/netronome/nfp/nfp_port.h
index 90ae053f5c07..09c55ca2371a 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.h
@@ -129,6 +129,8 @@ int nfp_net_refresh_eth_port(struct nfp_port *port);
 void nfp_net_refresh_port_table(struct nfp_port *port);
 int nfp_net_refresh_port_table_sync(struct nfp_pf *pf);
 
+int nfp_devlink_port_init(struct nfp_app *app, struct nfp_port *port);
+void nfp_devlink_port_clean(struct nfp_port *port);
 int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port);
 void nfp_devlink_port_unregister(struct nfp_port *port);
 
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 0/8] devlink: add PF and VF port flavours
From: Jakub Kicinski @ 2019-02-26 18:24 UTC (permalink / raw)
  To: davem, jiri; +Cc: oss-drivers, netdev, Jakub Kicinski

Hi!

This series brings port flavours for SR-IOV NICs.

First patch reshuffles nfp port registration to make the
port info available at netdev registration time. Next
the VF and PF port flavours are added to the core and
the nfp driver. The existing port information (split)
is now physical port-specific.

The currently deployed NFPs have a single PF per PCIe
link which means multiport cards have multiple logical
ports (netdevs) per function. This forces us to add
a notion of a PCIe subport (implemented in patches
4 and 5).

With that in place nfp driver can now switch to using
the devlink_port_get_phys_port_name() helper (patch 6).

Next two patches add a notion of a peer netdev, this
is the netdev "on the other side of the link". It can
be used to inform users which PF and VF corresponds
to which port/"representor" without the need for bus
info parsing.

Last but not least - I fix some kdoc warnings :)

Jakub Kicinski (8):
  nfp: split devlink port init from registration
  devlink: add PF and VF port flavours
  nfp: register devlink ports of all reprs
  devlink: allow subports on devlink PCI ports
  nfp: switch to devlink_port_get_phys_port_name()
  devlink: introduce port's peer netdevs
  nfp: expose PF peer netdevs
  devlink: fix kdoc

 drivers/net/ethernet/netronome/nfp/abm/main.c |   1 +
 .../net/ethernet/netronome/nfp/flower/main.c  |   1 +
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  60 ++++++-
 .../net/ethernet/netronome/nfp/nfp_net_main.c |  17 +-
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |  16 +-
 drivers/net/ethernet/netronome/nfp/nfp_port.c |  33 +---
 drivers/net/ethernet/netronome/nfp/nfp_port.h |   4 +
 include/net/devlink.h                         |  41 ++++-
 include/uapi/linux/devlink.h                  |   9 ++
 net/core/devlink.c                            | 153 +++++++++++++++---
 10 files changed, 268 insertions(+), 67 deletions(-)

-- 
2.19.2


^ permalink raw reply

* Re: [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: Michael Chan @ 2019-02-26 18:22 UTC (permalink / raw)
  To: David Miller
  Cc: Mahesh Bandewar (महेश बंडेवार),
	Eric Dumazet, Netdev, Daniel Axtens
In-Reply-To: <20190226.091324.1656342529138987636.davem@davemloft.net>

On Tue, Feb 26, 2019 at 9:13 AM David Miller <davem@davemloft.net> wrote:
>
> From: Michael Chan <michael.chan@broadcom.com>
> Date: Tue, 26 Feb 2019 05:56:41 -0500
>
> > There have been reports of oversize UDP packets being sent to the
> > driver to be transmitted, causing error conditions.  The issue is
> > likely caused by the dst of the SKB switching between 'lo' with
> > 64K MTU and the hardware device with a smaller MTU.  Patches are
> > being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
> > issue.
> >
> > Separately, we should add a length check in validate_xmit_skb()
> > to drop these oversize packets before they reach the driver.
> > This patch only validates non-TSO packets.  Complete validation
> > of segmented TSO packet size will probably be too slow.
> >
> > Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>
> Anything which changes the dst of an SKB really is responsible for
> fixing up whatever became "incompatible" in the new path.
>
> So like Eric I want to see this out of the fast path.

Ok.  In the meantime, will you take a 2-line bnxt_en patch that will
prevent this issue in kernel 5.0?

^ permalink raw reply

* Re: [PATCH] x86, retpolines: raise limit for generating indirect calls from switch-case
From: Björn Töpel @ 2019-02-26 18:20 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Daniel Borkmann, tglx, LKML, Netdev, David S . Miller,
	Björn Töpel, Magnus Karlsson, Alexei Starovoitov,
	Peter Zijlstra, David Woodhouse, Andy Lutomirski, Borislav Petkov,
	Linus Torvalds
In-Reply-To: <20190222083131.5a141e23@carbon>

On Fri, 22 Feb 2019 at 08:32, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> On Thu, 21 Feb 2019 23:19:41 +0100
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> > Recent work on XDP from Björn and Magnus additionally found that
> > manually transforming the XDP return code switch statement with
> > more than 5 cases into if-else combination would result in a
> > considerable speedup in XDP layer due to avoidance of indirect
> > calls in CONFIG_RETPOLINE enabled builds. On i40e driver with
> > XDP prog attached, a 20-26% speedup has been observed [0]. Aside
> > from XDP, there are many other places later in the networking
> > stack's critical path with similar switch-case processing. Rather
> > than fixing every XDP-enabled driver and locations in stack by
> > hand, it would be good to instead raise the limit where gcc would
> > emit expensive indirect calls from the switch under retpolines
>
> I'm very happy to see this.  Thanks to Björn for finding, analyzing and
> providing hand-coded-if-else code that demonstrated the performance
> issue for XDP.  But I do think this GCC case-values-threshold param is
> a better and more generic solution to the issue we observed and
> measured in XDP land. And hopefully other parts of the network stack
> and kernel will also benefit.
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Thanks for following up on this Daniel,

I definitely prefer a switch-statement over the if-else-messiness in
this context. Thanks for doing the deep-dive, Daniel!

FWIW,
Acked-by: Björn Töpel <bjorn.topel@intel.com>

> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: Daniel Borkmann @ 2019-02-26 18:12 UTC (permalink / raw)
  To: Martin Lau, zerons
  Cc: ast@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190226173355.chm6gwvwyer57yco@kafai-mbp.dhcp.thefacebook.com>

On 02/26/2019 06:33 PM, Martin Lau wrote:
> On Tue, Feb 26, 2019 at 10:15:37PM +0800, zerons wrote:
>> [ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
>>
>> In bpf/syscall.c, bpf_map_get_fd_by_id() use bpf_map_inc_not_zero() to increase
>> the refcount, both map->refcnt and map->usercnt. Then, if bpf_map_new_fd() fails,
>> should handle map->usercnt too.
> Good catch!  Thanks for the fix.

+1

> Fixes: bd5f5f4ecb78 ("bpf: Add BPF_MAP_GET_FD_BY_ID")
> Acked-by: Martin KaFai Lau <kafai@fb.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH v2 net] net: phy: phylink: fix uninitialized variable in phylink_get_mac_state
From: Heiner Kallweit @ 2019-02-26 18:08 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: David Miller, Andrew Lunn, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <20190226084313.v3uhd6yjmovzz24j@shell.armlinux.org.uk>

On 26.02.2019 09:43, Russell King - ARM Linux admin wrote:
> On Tue, Feb 26, 2019 at 08:25:41AM +0100, Heiner Kallweit wrote:
>> When debugging an issue I found implausible values in state->pause.
>> Reason in that state->pause isn't initialized and later only single
>> bits are changed. Also the struct itself isn't initialized in
>> phylink_resolve(). So better initialize state->pause.
> 
> mac_link_state() is expected to always set this, but this is safer.
> 
I've seen cases where mac_link_state() just initializes the relevant
bits. Therefore there's no functional problem, but it's not nice
(and disturbs in debug output) if all other bits have a random value.

> Maybe also set state->speed to SPEED_UNKNOWN, state->duplex to
> DUPLEX_UNKNOWN and state->an_complete to zero?
> 
OK. Will include this in a v3.

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


^ permalink raw reply

* [PATCH net-next 5/5] tcp: remove tcp_queue argument from tso_fragment()
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190226174913.18824-1-edumazet@google.com>

tso_fragment() is only called for packets still in write queue.

Remove the tcp_queue parameter to make this more obvious,
even if the comment clearly states this.

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

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 91f5cc5a3f88953da1043d051af9c5971c6006de..4522579aaca2d24b7be61abac658a3c8cbd2b960 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1846,17 +1846,17 @@ static bool tcp_snd_wnd_test(const struct tcp_sock *tp,
  * know that all the data is in scatter-gather pages, and that the
  * packet has never been sent out before (and thus is not cloned).
  */
-static int tso_fragment(struct sock *sk, enum tcp_queue tcp_queue,
-			struct sk_buff *skb, unsigned int len,
+static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
 			unsigned int mss_now, gfp_t gfp)
 {
-	struct sk_buff *buff;
 	int nlen = skb->len - len;
+	struct sk_buff *buff;
 	u8 flags;
 
 	/* All of a TSO frame must be composed of paged data.  */
 	if (skb->len != skb->data_len)
-		return tcp_fragment(sk, tcp_queue, skb, len, mss_now, gfp);
+		return tcp_fragment(sk, TCP_FRAG_IN_WRITE_QUEUE,
+				    skb, len, mss_now, gfp);
 
 	buff = sk_stream_alloc_skb(sk, 0, gfp, true);
 	if (unlikely(!buff))
@@ -1892,7 +1892,7 @@ static int tso_fragment(struct sock *sk, enum tcp_queue tcp_queue,
 
 	/* Link BUFF into the send queue. */
 	__skb_header_release(buff);
-	tcp_insert_write_queue_after(skb, buff, sk, tcp_queue);
+	tcp_insert_write_queue_after(skb, buff, sk, TCP_FRAG_IN_WRITE_QUEUE);
 
 	return 0;
 }
@@ -2391,8 +2391,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
 						    nonagle);
 
 		if (skb->len > limit &&
-		    unlikely(tso_fragment(sk, TCP_FRAG_IN_WRITE_QUEUE,
-					  skb, limit, mss_now, gfp)))
+		    unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
 			break;
 
 		if (tcp_small_queue_check(sk, skb, 0))
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* [PATCH net-next 4/5] tcp: use tcp_md5_needed for timewait sockets
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190226174913.18824-1-edumazet@google.com>

This might speedup tcp_twsk_destructor() a bit,
avoiding a cache line miss.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_minisocks.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 182595e2d40ff5b48cd6d095d4bd97b4cb26d3c0..79900f783e0d8a713712b0bf47700f16eebab4db 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -294,12 +294,15 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
 		 * so the timewait ack generating code has the key.
 		 */
 		do {
-			struct tcp_md5sig_key *key;
 			tcptw->tw_md5_key = NULL;
-			key = tp->af_specific->md5_lookup(sk, sk);
-			if (key) {
-				tcptw->tw_md5_key = kmemdup(key, sizeof(*key), GFP_ATOMIC);
-				BUG_ON(tcptw->tw_md5_key && !tcp_alloc_md5sig_pool());
+			if (static_branch_unlikely(&tcp_md5_needed)) {
+				struct tcp_md5sig_key *key;
+
+				key = tp->af_specific->md5_lookup(sk, sk);
+				if (key) {
+					tcptw->tw_md5_key = kmemdup(key, sizeof(*key), GFP_ATOMIC);
+					BUG_ON(tcptw->tw_md5_key && !tcp_alloc_md5sig_pool());
+				}
 			}
 		} while (0);
 #endif
@@ -338,10 +341,12 @@ EXPORT_SYMBOL(tcp_time_wait);
 void tcp_twsk_destructor(struct sock *sk)
 {
 #ifdef CONFIG_TCP_MD5SIG
-	struct tcp_timewait_sock *twsk = tcp_twsk(sk);
+	if (static_branch_unlikely(&tcp_md5_needed)) {
+		struct tcp_timewait_sock *twsk = tcp_twsk(sk);
 
-	if (twsk->tw_md5_key)
-		kfree_rcu(twsk->tw_md5_key, rcu);
+		if (twsk->tw_md5_key)
+			kfree_rcu(twsk->tw_md5_key, rcu);
+	}
 #endif
 }
 EXPORT_SYMBOL_GPL(tcp_twsk_destructor);
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* [PATCH net-next 3/5] tcp: convert tcp_md5_needed to static_branch API
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190226174913.18824-1-edumazet@google.com>

We prefer static_branch_unlikely() over static_key_false() these days.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h     | 4 ++--
 net/ipv4/tcp.c        | 2 +-
 net/ipv4/tcp_ipv4.c   | 2 +-
 net/ipv4/tcp_output.c | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d66ac76a33d1298e48bb9dcef217beaedc8a7968..68ee02523b873db037aa9d6707edb14592e763fd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1558,7 +1558,7 @@ struct tcp_md5sig_key *tcp_v4_md5_lookup(const struct sock *sk,
 
 #ifdef CONFIG_TCP_MD5SIG
 #include <linux/jump_label.h>
-extern struct static_key tcp_md5_needed;
+extern struct static_key_false tcp_md5_needed;
 struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk,
 					   const union tcp_md5_addr *addr,
 					   int family);
@@ -1567,7 +1567,7 @@ tcp_md5_do_lookup(const struct sock *sk,
 		  const union tcp_md5_addr *addr,
 		  int family)
 {
-	if (!static_key_false(&tcp_md5_needed))
+	if (!static_branch_unlikely(&tcp_md5_needed))
 		return NULL;
 	return __tcp_md5_do_lookup(sk, addr, family);
 }
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e2fa6eb9f81aa0244ea05d99e2870a3d9294673b..ad07dd71063da09843ccfbd3e00d3f41567e1205 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3699,7 +3699,7 @@ bool tcp_alloc_md5sig_pool(void)
 		if (!tcp_md5sig_pool_populated) {
 			__tcp_alloc_md5sig_pool();
 			if (tcp_md5sig_pool_populated)
-				static_key_slow_inc(&tcp_md5_needed);
+				static_branch_inc(&tcp_md5_needed);
 		}
 
 		mutex_unlock(&tcp_md5sig_mutex);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 4010ae3644f3101d01ee984c21256ca381588a3e..831d844a27ca20b52d590e17cb3eb2b2f8a697ce 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -973,7 +973,7 @@ static void tcp_v4_reqsk_destructor(struct request_sock *req)
  * We need to maintain these in the sk structure.
  */
 
-struct static_key tcp_md5_needed __read_mostly;
+DEFINE_STATIC_KEY_FALSE(tcp_md5_needed);
 EXPORT_SYMBOL(tcp_md5_needed);
 
 /* Find the Key structure for an address.  */
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index e72aa0ff5785f177eb28ba8a3e4714997e58f35b..91f5cc5a3f88953da1043d051af9c5971c6006de 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -597,7 +597,7 @@ static unsigned int tcp_syn_options(struct sock *sk, struct sk_buff *skb,
 
 	*md5 = NULL;
 #ifdef CONFIG_TCP_MD5SIG
-	if (static_key_false(&tcp_md5_needed) &&
+	if (static_branch_unlikely(&tcp_md5_needed) &&
 	    rcu_access_pointer(tp->md5sig_info)) {
 		*md5 = tp->af_specific->md5_lookup(sk, sk);
 		if (*md5) {
@@ -734,7 +734,7 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
 
 	*md5 = NULL;
 #ifdef CONFIG_TCP_MD5SIG
-	if (static_key_false(&tcp_md5_needed) &&
+	if (static_branch_unlikely(&tcp_md5_needed) &&
 	    rcu_access_pointer(tp->md5sig_info)) {
 		*md5 = tp->af_specific->md5_lookup(sk, sk);
 		if (*md5) {
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* [PATCH net-next 2/5] tcp: get rid of __tcp_add_write_queue_tail()
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190226174913.18824-1-edumazet@google.com>

This helper is only used from tcp_add_write_queue_tail(), and does
not make the code more readable.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 813042c08dc5a7438279821890439b02490391d1..d66ac76a33d1298e48bb9dcef217beaedc8a7968 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1716,14 +1716,9 @@ static inline bool tcp_rtx_and_write_queues_empty(const struct sock *sk)
 	return tcp_rtx_queue_empty(sk) && tcp_write_queue_empty(sk);
 }
 
-static inline void __tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
-{
-	__skb_queue_tail(&sk->sk_write_queue, skb);
-}
-
 static inline void tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
 {
-	__tcp_add_write_queue_tail(sk, skb);
+	__skb_queue_tail(&sk->sk_write_queue, skb);
 
 	/* Queue it, remembering where we must start sending. */
 	if (sk->sk_write_queue.next == skb)
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* [PATCH net-next 1/5] tcp: get rid of tcp_check_send_head()
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190226174913.18824-1-edumazet@google.com>

This helper is used only once, and its name is no longer relevant.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 6 ------
 net/ipv4/tcp.c    | 3 ++-
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index a6e0355921e1d0cf6df3bb20fba8b3d4e9d9ef08..813042c08dc5a7438279821890439b02490391d1 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1716,12 +1716,6 @@ static inline bool tcp_rtx_and_write_queues_empty(const struct sock *sk)
 	return tcp_rtx_queue_empty(sk) && tcp_write_queue_empty(sk);
 }
 
-static inline void tcp_check_send_head(struct sock *sk, struct sk_buff *skb_unlinked)
-{
-	if (tcp_write_queue_empty(sk))
-		tcp_chrono_stop(sk, TCP_CHRONO_BUSY);
-}
-
 static inline void __tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
 {
 	__skb_queue_tail(&sk->sk_write_queue, skb);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 769508c75dce5a39bd78766d801ca01abb77f4bb..e2fa6eb9f81aa0244ea05d99e2870a3d9294673b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1412,7 +1412,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 		/* It is the one place in all of TCP, except connection
 		 * reset, where we can be unlinking the send_head.
 		 */
-		tcp_check_send_head(sk, skb);
+		if (tcp_write_queue_empty(sk))
+			tcp_chrono_stop(sk, TCP_CHRONO_BUSY);
 		sk_wmem_free_skb(sk, skb);
 	}
 
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* [PATCH net-next 0/5] tcp: cleanups for linux-5.1
From: Eric Dumazet @ 2019-02-26 17:49 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

This small patch series cleanups few things, and add a small
timewait optimization for hosts not using md5.

Eric Dumazet (5):
  tcp: get rid of tcp_check_send_head()
  tcp: get rid of __tcp_add_write_queue_tail()
  tcp: convert tcp_md5_needed to static_branch API
  tcp: use tcp_md5_needed for timewait sockets
  tcp: remove tcp_queue argument from tso_fragment()

 include/net/tcp.h        | 17 +++--------------
 net/ipv4/tcp.c           |  5 +++--
 net/ipv4/tcp_ipv4.c      |  2 +-
 net/ipv4/tcp_minisocks.c | 21 +++++++++++++--------
 net/ipv4/tcp_output.c    | 17 ++++++++---------
 5 files changed, 28 insertions(+), 34 deletions(-)

-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply

* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink
From: Petr Machata @ 2019-02-26 17:45 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: davem@davemloft.net, netdev@vger.kernel.org,
	dsa@cumulusnetworks.com, sd@queasysnail.net,
	johannes@sipsolutions.net
In-Reply-To: <CAJieiUhKFrhoMcy-Bz1JmWEQ=8OooeZpb_sbD6zcKwotfJooBQ@mail.gmail.com>


Roopa Prabhu <roopa@cumulusnetworks.com> writes:

> On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote:
>>
>> Many of the flags for which you call this don't actually have the else
>> branch. However I suspect there are no good reasons not to allow
>> resetting a flag.
>>
>> Reviewed-by: Petr Machata <petrm@mellanox.com>
>
> yes, correct, that was intentional.
> also, I am not sure what is the best way to support reseting of a NLA_FLAG.
> Absence of the flag attribute in the request cannot be the reason for
> resetting or clearing the flag.

Yeah, I was thinking about that. It looks like this would need a new set
of u8 attributes :-(

> None of the NLA_FLAG attributes support changing the flag today. This
> patch does not change that.

^ permalink raw reply

* Re: [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: Martin Lau @ 2019-02-26 17:33 UTC (permalink / raw)
  To: zerons
  Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1551190537-28694-1-git-send-email-sironhide0null@gmail.com>

On Tue, Feb 26, 2019 at 10:15:37PM +0800, zerons wrote:
> [ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
> 
> In bpf/syscall.c, bpf_map_get_fd_by_id() use bpf_map_inc_not_zero() to increase
> the refcount, both map->refcnt and map->usercnt. Then, if bpf_map_new_fd() fails,
> should handle map->usercnt too.
Good catch!  Thanks for the fix.

Fixes: bd5f5f4ecb78 ("bpf: Add BPF_MAP_GET_FD_BY_ID")
Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* Re: [PATCH v1 iproute2-next 3/4] rdma: add 'link add/delete' commands
From: Steve Wise @ 2019-02-26 17:24 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223094311.GP23561@mtr-leonro.mtl.com>


On 2/23/2019 3:43 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:12AM -0800, Steve Wise wrote:
>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma
>> device to a netdev interface.
>>
>> EG:
>>
>> rdma link add rxe_eth0 type rxe netdev eth0
>> rdma link delete rxe_eth0
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>>  rdma/link.c  | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  rdma/rdma.h  |  1 +
>>  rdma/utils.c |  2 +-
>>  3 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/rdma/link.c b/rdma/link.c
>> index c064be627be2..afaf19663728 100644
>> --- a/rdma/link.c
>> +++ b/rdma/link.c
>> @@ -14,6 +14,9 @@
>>  static int link_help(struct rd *rd)
>>  {
>>  	pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename);
>> +	pr_out("Usage: %s link add NAME type TYPE netdev NETDEV\n",
>> +	       rd->filename);
>> +	pr_out("Usage: %s link delete NAME\n", rd->filename);
>>  	return 0;
>>  }
>>
>> @@ -341,10 +344,74 @@ static int link_show(struct rd *rd)
>>  	return rd_exec_link(rd, link_one_show, true);
>>  }
>>
>> +static int link_add(struct rd *rd)
>> +{
>> +	char *name;
>> +	char *type = NULL;
>> +	char *dev = NULL;
>> +	uint32_t seq;
>> +
>> +	if (rd_no_arg(rd)) {
>> +		pr_err("No link name was supplied\n");
> I think that it is better to have instruction message and not error
> message: "Please provide ...".


Ok.  Perhaps a new utility rd_exec_require_link() can be created?


>> +		return -EINVAL;
>> +	}
>> +	name = rd_argv(rd);
>> +	rd_arg_inc(rd);
>> +	while (!rd_no_arg(rd)) {
>> +		if (rd_argv_match(rd, "type")) {
>> +			rd_arg_inc(rd);
>> +			type = rd_argv(rd);
>> +		} else if (rd_argv_match(rd, "netdev")) {
>> +			rd_arg_inc(rd);
>> +			dev = rd_argv(rd);
>> +		} else {
>> +			pr_err("Invalid parameter %s\n", rd_argv(rd));
>> +			return -EINVAL;
>> +		}
>> +		rd_arg_inc(rd);
> Please use chains of struct rd_cmd and rd_exec_cmd() instead of
> open-coding parser.


Ok.  Like your recently merged series did...


>> +	}
>> +	if (!type) {
>> +		pr_err("No type was supplied\n");
>> +		return -EINVAL;
> General parser handle it.


Ok.


>> +	}
>> +	if (!dev) {
>> +		pr_err("No net device was supplied\n");
>> +		return -EINVAL;
>> +	}
> rd_exec_require_dev() ???


Looks like I can use that.  I'll try it.


Thanks!


Steve.



^ permalink raw reply

* Re: [PATCH net-next] tc-testing: gitignore, ignore local tdc config file
From: David Miller @ 2019-02-26 17:20 UTC (permalink / raw)
  To: vladbu; +Cc: netdev, jhs, shuah, lucasb
In-Reply-To: <20190226153709.17587-1-vladbu@mellanox.com>

From: Vlad Buslov <vladbu@mellanox.com>
Date: Tue, 26 Feb 2019 17:37:09 +0200

> Comment in tdc_config.py recommends putting customizations in
> tdc_config_local.py file that wasn't included in gitignore. Add the local
> config file to gitignore.
> 
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: sched: fix typo in walker_check_empty()
From: David Miller @ 2019-02-26 17:20 UTC (permalink / raw)
  To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190226153440.17349-1-vladbu@mellanox.com>

From: Vlad Buslov <vladbu@mellanox.com>
Date: Tue, 26 Feb 2019 17:34:40 +0200

> Function walker_check_empty() incorrectly verifies that tp pointer is not
> NULL, instead of actual filter pointer. Fix conditional to check the right
> pointer. Adjust filter pointer naming accordingly to other cls API
> functions.
> 
> Fixes: 6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Steve Wise @ 2019-02-26 17:19 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223093122.GO23561@mtr-leonro.mtl.com>


On 2/23/2019 3:31 AM, Leon Romanovsky wrote:
> On Sat, Feb 23, 2019 at 11:26:15AM +0200, Leon Romanovsky wrote:
>> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
>>> This function sends the constructed netlink message and then
>>> receives the response, displaying any error text.
>>>
>>> Change 'rdma dev set' to use it.
>>>
>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>>> ---
>>>  rdma/dev.c   |  2 +-
>>>  rdma/rdma.h  |  1 +
>>>  rdma/utils.c | 21 +++++++++++++++++++++
>>>  3 files changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rdma/dev.c b/rdma/dev.c
>>> index 60ff4b31e320..d2949c378f08 100644
>>> --- a/rdma/dev.c
>>> +++ b/rdma/dev.c
>>> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
>>>  	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
>>>  	mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>>>
>>> -	return rd_send_msg(rd);
>>> +	return rd_sendrecv_msg(rd, seq);
>>>  }
>>>
>>>  static int dev_one_set(struct rd *rd)
>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>>> index 547bb5749a39..20be2f12c4f8 100644
>>> --- a/rdma/rdma.h
>>> +++ b/rdma/rdma.h
>>> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
>>>   */
>>>  int rd_send_msg(struct rd *rd);
>>>  int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
>>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
>>>  void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
>>>  int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
>>>  int rd_attr_cb(const struct nlattr *attr, void *data);
>>> diff --git a/rdma/utils.c b/rdma/utils.c
>>> index 069d44fece10..a6f2826c9605 100644
>>> --- a/rdma/utils.c
>>> +++ b/rdma/utils.c
>>> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
>>>  	return ret;
>>>  }
>>>
>>> +static int null_cb(const struct nlmsghdr *nlh, void *data)
>>> +{
>>> +	return MNL_CB_OK;
>>> +}
>>> +
>>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = rd_send_msg(rd);
>>> +	if (ret) {
>>> +		perror(NULL);
>> This is more or less already done in rd_send_msg() and that function
>> prints something in case of execution error. So the missing piece
>> is to update rd_recv_msg(), so all places will "magically" print errors
>> and not only dev_set_name().
>>
>>> +		goto out;
>>> +	}
>>> +	ret = rd_recv_msg(rd, null_cb, rd, seq);
> Will this "null_cb" work for all send/recv flows or only in flows where
> response can be error only? 


Only those flows where no nl attributes are expected to be returned.


> Will we need this recv_msg if we implement
> extack support?


I'm not sure how extack works.  Do you know?

Thanks!

Steve.



^ permalink raw reply

* Re: [PATCH v1 iproute2-next 2/4] Sync up rdma_netlink.h
From: Steve Wise @ 2019-02-26 17:15 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092820.GN23561@mtr-leonro.mtl.com>


On 2/23/2019 3:28 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:07AM -0800, Steve Wise wrote:
>> Pull in the latest rdma_netlink.h to get the RDMA_NLDEV_CMD_NEWLINK /
>> RDMA_NLDEV_CMD_DELLINK API.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>>  rdma/include/uapi/rdma/rdma_netlink.h | 74 +++++++++++++++++++++++++++--------
>>  1 file changed, 58 insertions(+), 16 deletions(-)
>>
> If my series go first, this patch won't be needed.
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

Some changes are still needed because your iproute2 series that was
recently merged didn't include the kernel NEWLINK/DELLINK changes.   But
I've rebased this and all the IWPM changes are already there as part of
your series.

Thanks,

Steve.


^ permalink raw reply

* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Marc Zyngier @ 2019-02-26 17:14 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
	Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
	Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
	<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
	linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <CAKv+Gu_=bQyVZqgUnJm9moCcaCcpDrag6NWGEpaR65=pMiTFJQ@mail.gmail.com>

On 26/02/2019 16:21, Ard Biesheuvel wrote:
> On Mon, 25 Feb 2019 at 15:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>> Hi Ard,
>>
>> On 25/02/2019 12:45, Ard Biesheuvel wrote:
>>> On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>
>>>> For quite some time, I wondered why the PCI mwifiex device built in my
>>>> Chromebook was unable to use the good old legacy interrupts. But as MSIs
>>>> were working fine, I never really bothered investigating. I finally had a
>>>> look, and the result isn't very pretty.
>>>>
>>>> On this machine (rk3399-based kevin), the wake-up interrupt is described as
>>>> such:
>>>>
>>>> &pci_rootport {
>>>>         mvl_wifi: wifi@0,0 {
>>>>                 compatible = "pci1b4b,2b42";
>>>>                 reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>>>>                        0x83010000 0x0 0x00100000 0x0 0x00100000>;
>>>>                 interrupt-parent = <&gpio0>;
>>>>                 interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>>>>                 pinctrl-names = "default";
>>>>                 pinctrl-0 = <&wlan_host_wake_l>;
>>>>                 wakeup-source;
>>>>         };
>>>> };
>>>>
>>>> Note how the interrupt is part of the properties directly attached to the
>>>> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
>>>> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
>>>> altogether (Yay for the broken design!). This is in total violation of the
>>>> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
>>>> specifiers describe the PCI device interrupts, and must obey the
>>>> INT-{A,B,C,D} mapping. Oops!
>>>>
>>>
>>> The mapping of legacy PCIe INTx interrupts onto wired system
>>> interrupts is a property of the PCIe host controller, not of a
>>> particular PCIe device. So I would argue that the code is broken here
>>> as well: it should never attempt to interpret 'interrupt' properties
>>> at the PCI device level as having any bearing on how legacy interrupts
>>> are routed.
>>
>> OpenFirmware says that this node contains the interrupt number of the
>> device (4.1.1. Open Firmware-defined Properties for Child Nodes). The
>> trick is that this property is generated *from* the device, and not set
>> in stone.
>>
>> DT, on the other hand, takes whatever is described there and uses it as
>> the gospel to configure the OS, no matter how the PCI device is actually
>> configured. If the two don't match (like in this case), things break.
>> This is the "DT describes the HW" mantra, for (sometimes) better or
>> (more generally) worse.
>>
>> What the DT code does is to interpret the whole interrupt specifier,
>> *including the interrupt-parent*. And that feels wrong. It should always
>> be in the context of the host controller. But on the other side, the DT
>> code is not in the business of validating the DT either...
>>
>> It outlines one thing: If you have to interpret per-device PCI
>> properties from DT, you're in for serious trouble. I should get some
>> better HW.
>>
> 
> Yeah, it obviously makes no sense at all for the interrupt parent of a
> PCI device to deviate from the host bridge's interrupt parent, and
> it's quite unfortunate that we can't simply ban it now that the cat is
> out of the bag already.
> 
> Arguably, the wake up widget is not part of the PCI device, but I have
> no opinion as to whether it is better modeling it as a sub device as
> you are proposing or as an entirely separate device referenced via a
> phandle.

It is not that clear. The widget seems to be an integral part of the
device, as it is the same basic IP that is used for SDIO and USB.

It looks like the good old pre-PCI-2.2 days, where you had to have a
separate cable between your network card and the base-board for the
wake-up interrupt to be delivered. Starting with PCI-2.2, the bus can
carry the signal just fine. With PCIe, it should just be an interrupt
TLP sent to the RC, but that's obviously not within the capabilities of
the HW.

Anyway, it'd be good if the Marvell people could chime in and let us
know how they'd prefer to handle this.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Steve Wise @ 2019-02-26 17:13 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092615.GM23561@mtr-leonro.mtl.com>


On 2/23/2019 3:26 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
>> This function sends the constructed netlink message and then
>> receives the response, displaying any error text.
>>
>> Change 'rdma dev set' to use it.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>>  rdma/dev.c   |  2 +-
>>  rdma/rdma.h  |  1 +
>>  rdma/utils.c | 21 +++++++++++++++++++++
>>  3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/rdma/dev.c b/rdma/dev.c
>> index 60ff4b31e320..d2949c378f08 100644
>> --- a/rdma/dev.c
>> +++ b/rdma/dev.c
>> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
>>  	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
>>  	mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>>
>> -	return rd_send_msg(rd);
>> +	return rd_sendrecv_msg(rd, seq);
>>  }
>>
>>  static int dev_one_set(struct rd *rd)
>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>> index 547bb5749a39..20be2f12c4f8 100644
>> --- a/rdma/rdma.h
>> +++ b/rdma/rdma.h
>> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
>>   */
>>  int rd_send_msg(struct rd *rd);
>>  int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
>>  void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
>>  int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
>>  int rd_attr_cb(const struct nlattr *attr, void *data);
>> diff --git a/rdma/utils.c b/rdma/utils.c
>> index 069d44fece10..a6f2826c9605 100644
>> --- a/rdma/utils.c
>> +++ b/rdma/utils.c
>> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
>>  	return ret;
>>  }
>>
>> +static int null_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> +	return MNL_CB_OK;
>> +}
>> +
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
>> +{
>> +	int ret;
>> +
>> +	ret = rd_send_msg(rd);
>> +	if (ret) {
>> +		perror(NULL);
> This is more or less already done in rd_send_msg() and that function
> prints something in case of execution error. So the missing piece
> is to update rd_recv_msg(), so all places will "magically" print errors
> and not only dev_set_name().

Yea ok.



^ permalink raw reply

* Re: [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: David Miller @ 2019-02-26 17:13 UTC (permalink / raw)
  To: michael.chan; +Cc: maheshb, edumazet, dja, netdev
In-Reply-To: <1551178601-16564-1-git-send-email-michael.chan@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>
Date: Tue, 26 Feb 2019 05:56:41 -0500

> There have been reports of oversize UDP packets being sent to the
> driver to be transmitted, causing error conditions.  The issue is
> likely caused by the dst of the SKB switching between 'lo' with
> 64K MTU and the hardware device with a smaller MTU.  Patches are
> being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
> issue.
> 
> Separately, we should add a length check in validate_xmit_skb()
> to drop these oversize packets before they reach the driver.
> This patch only validates non-TSO packets.  Complete validation
> of segmented TSO packet size will probably be too slow.
> 
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Anything which changes the dst of an SKB really is responsible for
fixing up whatever became "incompatible" in the new path.

So like Eric I want to see this out of the fast path.

^ permalink raw reply

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

From: Leslie Monis <lesliemonis@gmail.com>
Date: Tue, 26 Feb 2019 15:53:31 +0530

> Fix the incorrect reference link to RFC 8033
> 
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: David Miller @ 2019-02-26 17:09 UTC (permalink / raw)
  To: Nicolas.Ferre
  Cc: f.fainelli, pthombar, netdev, andrew, hkallweit1, linux-kernel,
	rafalc, piotrs, jank, Claudiu.Beznea
In-Reply-To: <a0f7be10-ff9b-c11c-fde9-30876940d719@microchip.com>

From: <Nicolas.Ferre@microchip.com>
Date: Tue, 26 Feb 2019 09:23:04 +0000

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

Ok.

^ 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