Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 6/7] net: ethernet: bgmac: add NS2 support
From: Jon Mason @ 2016-11-02 16:53 UTC (permalink / raw)
  To: Scott Branden
  Cc: David Miller, Rob Herring, Mark Rutland, Florian Fainelli, rafal,
	bcm-kernel-feedback-list, netdev, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20161101210512.GB27191@broadcom.com>

On Tue, Nov 01, 2016 at 05:05:13PM -0400, Jon Mason wrote:
> On Tue, Nov 01, 2016 at 01:34:30PM -0700, Scott Branden wrote:
> > One change in this patch
> > 
> > On 16-11-01 01:04 PM, Jon Mason wrote:
> > >Add support for the variant of amac hardware present in the Broadcom
> > >Northstar2 based SoCs.  Northstar2 requires an additional register to be
> > >configured with the port speed/duplexity (NICPM).  This can be added to
> > >the link callback to hide it from the instances that do not use this.
> > >Also, clearing of the pending interrupts on init is required due to
> > >observed issues on some platforms.
> > >
> > >Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > >---
> > > drivers/net/ethernet/broadcom/bgmac-platform.c | 56 +++++++++++++++++++++++++-
> > > drivers/net/ethernet/broadcom/bgmac.c          |  3 ++
> > > drivers/net/ethernet/broadcom/bgmac.h          |  1 +
> > > 3 files changed, 58 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
> > >index aed5dc5..f6d48c7 100644
> > >--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
> > >+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
> > >@@ -14,12 +14,21 @@
> > > #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
> > >
> > > #include <linux/bcma/bcma.h>
> > >+#include <linux/brcmphy.h>
> > > #include <linux/etherdevice.h>
> > > #include <linux/of_address.h>
> > > #include <linux/of_mdio.h>
> > > #include <linux/of_net.h>
> > > #include "bgmac.h"
> > >
> > >+#define NICPM_IOMUX_CTRL		0x00000008
> > >+
> > >+#define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
> > >+#define NICPM_IOMUX_CTRL_SPD_SHIFT	10
> > >+#define NICPM_IOMUX_CTRL_SPD_10M	0
> > >+#define NICPM_IOMUX_CTRL_SPD_100M	1
> > >+#define NICPM_IOMUX_CTRL_SPD_1000M	2
> > >+
> > > static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
> > > {
> > > 	return readl(bgmac->plat.base + offset);
> > >@@ -87,12 +96,46 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
> > > 	WARN_ON(1);
> > > }
> > >
> > >+static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> > >+{
> > >+	struct bgmac *bgmac = netdev_priv(net_dev);
> > >+	u32 val;
> > >+
> > >+	if (!bgmac->plat.nicpm_base)
> > >+		return;
> > >+
> > >+	val = NICPM_IOMUX_CTRL_INIT_VAL;
> > >+	switch (bgmac->net_dev->phydev->speed) {
> > >+	default:
> > >+		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> > This should be dev_err
> 
> It should probably be netdev_err (and there are a few instances below
> that should probably be changed to netdev_err as well).

Actually, the other instances I referenced above should not be
netdev_err, as they are enountered before the netdev is created.  So,
dev_err is correct for them.

That being said, the original pr_err that Scott referenced should be
netdev_err (as it is encountered after the netdev is created).  v5 will
make that change.

Thanks,
Jon

> 
> Thanks,
> Jon
> 
> > >+	case SPEED_1000:
> > >+		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > >+		break;
> > >+	case SPEED_100:
> > >+		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > >+		break;
> > >+	case SPEED_10:
> > >+		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > >+		break;
> > >+	}
> > >+
> > >+	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> > >+
> > >+	bgmac_adjust_link(bgmac->net_dev);
> > >+}
> > >+
> > > static int platform_phy_connect(struct bgmac *bgmac)
> > > {
> > > 	struct phy_device *phy_dev;
> > >
> > >-	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> > >-					 bgmac_adjust_link);
> > >+	if (bgmac->plat.nicpm_base)
> > >+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > >+						 bgmac->dev->of_node,
> > >+						 bgmac_nicpm_speed_set);
> > >+	else
> > >+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > >+						 bgmac->dev->of_node,
> > >+						 bgmac_adjust_link);
> > > 	if (!phy_dev) {
> > > 		dev_err(bgmac->dev, "Phy connect failed\n");
> > > 		return -ENODEV;
> > >@@ -182,6 +225,14 @@ static int bgmac_probe(struct platform_device *pdev)
> > > 	if (IS_ERR(bgmac->plat.idm_base))
> > > 		return PTR_ERR(bgmac->plat.idm_base);
> > >
> > >+	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> > >+	if (regs) {
> > >+		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> > >+							       regs);
> > >+		if (IS_ERR(bgmac->plat.nicpm_base))
> > >+			return PTR_ERR(bgmac->plat.nicpm_base);
> > >+	}
> > >+
> > > 	bgmac->read = platform_bgmac_read;
> > > 	bgmac->write = platform_bgmac_write;
> > > 	bgmac->idm_read = platform_bgmac_idm_read;
> > >@@ -213,6 +264,7 @@ static int bgmac_remove(struct platform_device *pdev)
> > > static const struct of_device_id bgmac_of_enet_match[] = {
> > > 	{.compatible = "brcm,amac",},
> > > 	{.compatible = "brcm,nsp-amac",},
> > >+	{.compatible = "brcm,ns2-amac",},
> > > 	{},
> > > };
> > >
> > >diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> > >index 4584958..a805cc8 100644
> > >--- a/drivers/net/ethernet/broadcom/bgmac.c
> > >+++ b/drivers/net/ethernet/broadcom/bgmac.c
> > >@@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
> > > /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
> > > static void bgmac_chip_init(struct bgmac *bgmac)
> > > {
> > >+	/* Clear any erroneously pending interrupts */
> > >+	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> > >+
> > > 	/* 1 interrupt per received frame */
> > > 	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
> > >
> > >diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
> > >index ea52ac3..b1820ea 100644
> > >--- a/drivers/net/ethernet/broadcom/bgmac.h
> > >+++ b/drivers/net/ethernet/broadcom/bgmac.h
> > >@@ -463,6 +463,7 @@ struct bgmac {
> > > 		struct {
> > > 			void *base;
> > > 			void *idm_base;
> > >+			void *nicpm_base;
> > > 		} plat;
> > > 		struct {
> > > 			struct bcma_device *core;
> > >

^ permalink raw reply

* [PATCH net] ipv4: allow local fragmentation in ip_finish_output_gso()
From: Lance Richardson @ 2016-11-02 16:29 UTC (permalink / raw)
  To: netdev, fw, jtluka

Some configurations (e.g. geneve interface with default
MTU of 1500 over an ethernet interface with 1500 MTU) result
in the transmission of packets that exceed the configured MTU.
While this should be considered to be a "bad" configuration,
it is still allowed and should not result in the sending
of packets that exceed the configured MTU.

Fix by dropping the assumption in ip_finish_output_gso() that
locally originated gso packets will never need fragmentation.
Basic testing using iperf (observing CPU usage and bandwidth)
have shown no measurable performance impact for traffic not
requiring fragmentation.

Fixes: c7ba65d7b649 ("net: ip: push gso skb forwarding handling down the stack")
Reported-by: Jan Tluka <jtluka@redhat.com>
Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 net/ipv4/ip_output.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 03e7f73..4971401 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -239,11 +239,9 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk,
 	struct sk_buff *segs;
 	int ret = 0;
 
-	/* common case: fragmentation of segments is not allowed,
-	 * or seglen is <= mtu
+	/* common case: seglen is <= mtu
 	 */
-	if (((IPCB(skb)->flags & IPSKB_FRAG_SEGS) == 0) ||
-	      skb_gso_validate_mtu(skb, mtu))
+	if (skb_gso_validate_mtu(skb, mtu))
 		return ip_finish_output2(net, sk, skb);
 
 	/* Slowpath -  GSO segment length is exceeding the dst MTU.
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: Alexander Duyck @ 2016-11-02 16:06 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, John Fastabend, Michael S. Tsirkin,
	Shrijeet Mukherjee, Tom Herbert, Netdev, Shrijeet Mukherjee,
	roopa, nikolay
In-Reply-To: <20161102150100.6d32f281@redhat.com>

On Wed, Nov 2, 2016 at 7:01 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Fri, 28 Oct 2016 13:11:01 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
>> From: John Fastabend <john.fastabend@gmail.com>
>> Date: Fri, 28 Oct 2016 08:56:35 -0700
>>
>> > On 16-10-27 07:10 PM, David Miller wrote:
>> >> From: Alexander Duyck <alexander.duyck@gmail.com>
>> >> Date: Thu, 27 Oct 2016 18:43:59 -0700
>> >>
>> >>> On Thu, Oct 27, 2016 at 6:35 PM, David Miller <davem@davemloft.net> wrote:
>> >>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>> >>>> Date: Fri, 28 Oct 2016 01:25:48 +0300
>> >>>>
>> >>>>> On Thu, Oct 27, 2016 at 05:42:18PM -0400, David Miller wrote:
>> >>>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>> >>>>>> Date: Fri, 28 Oct 2016 00:30:35 +0300
>> >>>>>>
>> >>>>>>> Something I'd like to understand is how does XDP address the
>> >>>>>>> problem that 100Byte packets are consuming 4K of memory now.
>> >>>>>>
>> >>>>>> Via page pools.  We're going to make a generic one, but right now
>> >>>>>> each and every driver implements a quick list of pages to allocate
>> >>>>>> from (and thus avoid the DMA man/unmap overhead, etc.)
>> >>>>>
>> >>>>> So to clarify, ATM virtio doesn't attempt to avoid dma map/unmap
>> >>>>> so there should be no issue with that even when using sub/page
>> >>>>> regions, assuming DMA APIs support sub-page map/unmap correctly.
>> >>>>
>> >>>> That's not what I said.
>> >>>>
>> >>>> The page pools are meant to address the performance degradation from
>> >>>> going to having one packet per page for the sake of XDP's
>> >>>> requirements.
>> >>>>
>> >>>> You still need to have one packet per page for correct XDP operation
>> >>>> whether you do page pools or not, and whether you have DMA mapping
>> >>>> (or it's equivalent virutalization operation) or not.
>> >>>
>> >>> Maybe I am missing something here, but why do you need to limit things
>> >>> to one packet per page for correct XDP operation?  Most of the drivers
>> >>> out there now are usually storing something closer to at least 2
>> >>> packets per page, and with the DMA API fixes I am working on there
>> >>> should be no issue with changing the contents inside those pages since
>> >>> we won't invalidate or overwrite the data after the DMA buffer has
>> >>> been synchronized for use by the CPU.
>> >>
>> >> Because with SKB's you can share the page with other packets.
>> >>
>> >> With XDP you simply cannot.
>> >>
>> >> It's software semantics that are the issue.  SKB frag list pages
>> >> are read only, XDP packets are writable.
>> >>
>> >> This has nothing to do with "writability" of the pages wrt. DMA
>> >> mapping or cpu mappings.
>> >>
>> >
>> > Sorry I'm not seeing it either. The current xdp_buff is defined
>> > by,
>> >
>> >   struct xdp_buff {
>> >     void *data;
>> >     void *data_end;
>> >   };
>> >
>> > The verifier has an xdp_is_valid_access() check to ensure we don't go
>> > past data_end. The page for now at least never leaves the driver. For
>> > the work to get xmit to other devices working I'm still not sure I see
>> > any issue.
>>
>> I guess I can say that the packets must be "writable" until I'm blue
>> in the face but I'll say it again, semantically writable pages are a
>> requirement.  And if multiple packets share a page this requirement
>> is not satisfied.
>>
>> Also, we want to do several things in the future:
>>
>> 1) Allow push/pop of headers via eBPF code, which needs we need
>>    headroom.
>>
>> 2) Transparently zero-copy pass packets into userspace, basically
>>    the user will have a semi-permanently mapped ring of all the
>>    packet pages sitting in the RX queue of the device and the
>>    page pool associated with it.  This way we avoid all of the
>>    TLB flush/map overhead for the user's mapping of the packets
>>    just as we avoid the DMA map/unmap overhead.
>>
>> And that's just the beginninng.
>>
>> I'm sure others can come up with more reasons why we have this
>> requirement.
>
> I've tried to update the XDP documentation about the "Page per packet"
> requirement[1], fell free to correct below text:
>
> Page per packet
> ===============
>
> On RX many NIC drivers splitup a memory page, to share it for multiple
> packets, in-order to conserve memory.  Doing so complicates handling
> and accounting of these memory pages, which affects performance.
> Particularly the extra atomic refcnt handling needed for the page can
> hurt performance.

The atomic refcnt handling isn't a big deal.  It is easily worked
around by doing bulk updates.  The allocating and freeing of pages on
the other hand is quite expensive by comparison.

> XDP defines upfront a memory model where there is only one packet per
> page.  This simplifies page handling and open up for future
> extensions.

This is blatantly wrong.  It complicates page handling especially
since you are implying you aren't using the page count which implies a
new memory model like SLUB but not SLUB and hopefully including DMA
pools of some sort.  The fact is the page count tricks are much
cheaper than having to come up with your own page allocator.  In
addition having to share these with the stack means the network stack
and file system has to be updated to handle the special pages you are
sending that aren't using the page count.

> This requirement also (upfront) result in choosing not to support
> things like, jumpo-frames, LRO and generally packets split over
> multiple pages.

Same here.  Page per packet is only limiting if the final frame size
is larger than a page.  So for example on a 4K page I could setup an
82599 to support doing hardware LRO into a single 3K block which would
give you 2 frames per packet.  In the case of pages larger than 4k it
wouldn't be hard to support jumbo frames.  Where it gets messier is
the headroom requirements since I have yet to hear how that is going
to be communicated to the XPS programs.

> In the future, this strict memory model might be relaxed, but for now
> it is a strict requirement.  With a more flexible
> :ref:`ref_prog_negotiation` is might be possible to negotiate another
> memory model. Given some specific XDP use-case might not require this
> strict memory model.

Honestly, I think making this a hard requirement is going to hurt
overall performance for XDP.  I get that it is needed if you are
planning to support a use case where userspace is somehow sharing
buffers with the ring but I really think that it is getting the cart
ahead of the horse since in that case you should have a ring that is
completely reserved for that userspace application anyway and we
haven't figured out a good way to address that.  All of this page per
packet stuff just seems like premature optimization for a use case
that doesn't yet exist.

In the cases where packets are just getting sent into the network
stack with XDP as a bump in the wire I don't see the point in having
the entire page reserved for a single packet.  It seems like a fast
way to start leaking memory and eating up resources since every frame
going into the stack is going to end up with a truesize of over 4K(64K
in some cases) unless we introduce some sort of copy-break.

Anyway that is my $.02 on all this.

- Alex

^ permalink raw reply

* Re: [PATCH net-next 3/3] ipv6: on reassembly, record frag_max_size
From: Eric Dumazet @ 2016-11-02 15:55 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, jdorfman, davem, Willem de Bruijn
In-Reply-To: <1478098938-124108-4-git-send-email-willemdebruijn.kernel@gmail.com>

On Wed, 2016-11-02 at 11:02 -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> IP6CB and IPCB have a frag_max_size field. In IPv6 this field is
> filled in when packets are reassembled by the connection tracking
> code. Also fill in when reassembling in the input path, to expose
> it through cmsg IPV6_RECVFRAGSIZE in all cases.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH net-next 2/3] ipv6: add IPV6_RECVFRAGSIZE cmsg
From: Eric Dumazet @ 2016-11-02 15:53 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, jdorfman, davem, Willem de Bruijn
In-Reply-To: <1478098938-124108-3-git-send-email-willemdebruijn.kernel@gmail.com>

On Wed, 2016-11-02 at 11:02 -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> When reading a datagram or raw packet that arrived fragmented, expose
> the maximum fragment size if recorded to allow applications to
> estimate receive path MTU.

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH net-next 1/3] ipv4: add IP_RECVFRAGSIZE cmsg
From: Eric Dumazet @ 2016-11-02 15:52 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, jdorfman, davem, Willem de Bruijn
In-Reply-To: <1478098938-124108-2-git-send-email-willemdebruijn.kernel@gmail.com>

On Wed, 2016-11-02 at 11:02 -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> The IP stack records the largest fragment of a reassembled packet
> in IPCB(skb)->frag_max_size. When reading a datagram or raw packet
> that arrived fragmented, expose the value to allow applications to
> estimate receive path MTU.

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* RE: [PATCH RFC 0/2] ethtool: Add actual port speed reporting
From: Mintz, Yuval @ 2016-11-02 15:50 UTC (permalink / raw)
  To: Gal Pressman, netdev@vger.kernel.org, John W. Linville,
	Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings
In-Reply-To: <1478100914-23610-1-git-send-email-galp@mellanox.com>

> Sending RFC to get feedback for the following ethtool proposal:
> 
> In some cases such as virtual machines and multi functions (SR-IOV), the actual
> bandwidth exposed for each machine is not accurately shown in ethtool.
> Currently ethtool shows only physical port link speed.
> In our case we would like to show the virtual port operational link speed which
> in some cases is less than the physical port speed.
> 
> This will give users better visibility for the actual speed running on their device.
> 
> $ ethtool ens6
> ...
> Speed: 50000Mb/s
> Actual speed: 25000Mb/s

Not saying this is a bad thing, but where exactly is it listed that ethtool has
to show the physical port speed?
E.g., bnx2x shows the logical speed instead, and has been doing that for years.
[Perhaps that's a past wrongness, but that's how it goes].

And besides, one can argue that in the SR-IOV scenario the VF has no business
knowing the physical port speed.

^ permalink raw reply

* Re: [patch net-next 0/2] Fixes for raw diag sockets handling
From: Cyrill Gorcunov @ 2016-11-02 15:45 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev, Eric Dumazet, David S. Miller, Andrey Vagin,
	Stephen Hemminger
In-Reply-To: <ec9f3fbe-c95c-6e07-1263-29784d9ba361@cumulusnetworks.com>

On Wed, Nov 02, 2016 at 09:36:55AM -0600, David Ahern wrote:
> 
> Limited to raw sockets or are you looking at multiple spec options (dev, address, port)?
> 
> I have not seen issues with tcp or udp. Running:
> 
>     ss -aK 'dev == red' 
> 
> drops all sockets bound to device 'red' (or at least signaling the socket failure for the app to handle):

Limited to raw socket. I didn't modify lookup kernel code but use already existing helpers.
The tcp/udp sockets do use port value in lookup (iirc, don't have code under my hand
at moment), in turn raw lookup uses only net,raw-protocol, src/dst and device index.
In my test case the sokets were unconnected so the have no address but bound to
device and I hit mismatch. Then looking into inet matching code I found this weird
snippet I posted previously.

> 
> root@jessie4:~# ss -ap 'dev == red'
> Netid  State      Recv-Q Send-Q     Local Address:Port                      Peer Address:Port
> udp    UNCONN     0      0                  *%red:12345                                *:*                     users:(("vrf-test",pid=765,fd=3))
> tcp    LISTEN     0      1                  *%red:12345                                *:*                     users:(("vrf-test",pid=766,fd=3))
> tcp    ESTAB      0      0         10.100.1.4%red:ssh                       10.100.1.254:60298                 users:(("sshd",pid=738,fd=3))
> 
> root@jessie4:~# ss -aKp 'dev == red'
> Netid State      Recv-Q Send-Q                      Local Address:Port                                       Peer Address:Port
> udp   UNCONN     0      0                                   *%red:12345                                                 *:*                     users:(("vrf-test",pid=765,fd=3))
> tcp   LISTEN     0      1                                   *%red:12345                                                 *:*                     users:(("vrf-test",pid=766,fd=3))
> tcp   ESTAB      0      0                          10.100.1.4%red:ssh                                        10.100.1.254:60298                 users:(("sshd",pid=738,fd=3))
> 
> root@jessie4:~# ss -ap 'dev == red'
> Netid State      Recv-Q Send-Q                      Local Address:Port                                       Peer Address:Port

	Cyrill

^ permalink raw reply

* Re: [patch net-next 0/2] Fixes for raw diag sockets handling
From: David Ahern @ 2016-11-02 15:36 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: netdev, Eric Dumazet, David S. Miller, Andrey Vagin,
	Stephen Hemminger
In-Reply-To: <20161102152900.GC1852@uranus.lan>

On 11/2/16 9:29 AM, Cyrill Gorcunov wrote:
> On Wed, Nov 02, 2016 at 09:10:32AM -0600, David Ahern wrote:
>>> @__dif != 0 the match may return socket where sk_bound_dev_if = 0
>>> instead of completely matching one. Isn't it?
>>
>> yes. I recently added an exact_dif to the lookup for listener sockets
>> (see compute_score). Something like that could be added to INET_MATCH.
> 
> Seem so. I need to revisit this moment. Because with current lookup code
> iproute2 patches I made and been testing do not kill all sockets bound
> to particular device in one pass (because request from userspace asks
> for index 15 in my case but kernel return one with index 0). At first
> I thought I made a mistake in userspace code but once I added printk's
> into kernel I found that here some strange results over lookup.

Limited to raw sockets or are you looking at multiple spec options (dev, address, port)?

I have not seen issues with tcp or udp. Running:

    ss -aK 'dev == red' 

drops all sockets bound to device 'red' (or at least signaling the socket failure for the app to handle):

root@jessie4:~# ss -ap 'dev == red'
Netid  State      Recv-Q Send-Q     Local Address:Port                      Peer Address:Port
udp    UNCONN     0      0                  *%red:12345                                *:*                     users:(("vrf-test",pid=765,fd=3))
tcp    LISTEN     0      1                  *%red:12345                                *:*                     users:(("vrf-test",pid=766,fd=3))
tcp    ESTAB      0      0         10.100.1.4%red:ssh                       10.100.1.254:60298                 users:(("sshd",pid=738,fd=3))

root@jessie4:~# ss -aKp 'dev == red'
Netid State      Recv-Q Send-Q                      Local Address:Port                                       Peer Address:Port
udp   UNCONN     0      0                                   *%red:12345                                                 *:*                     users:(("vrf-test",pid=765,fd=3))
tcp   LISTEN     0      1                                   *%red:12345                                                 *:*                     users:(("vrf-test",pid=766,fd=3))
tcp   ESTAB      0      0                          10.100.1.4%red:ssh                                        10.100.1.254:60298                 users:(("sshd",pid=738,fd=3))

root@jessie4:~# ss -ap 'dev == red'
Netid State      Recv-Q Send-Q                      Local Address:Port                                       Peer Address:Port

^ permalink raw reply

* [PATCH RFC 2/2] net/mlx5e: Add support for ethtool get actual speed callback
From: Gal Pressman @ 2016-11-02 15:35 UTC (permalink / raw)
  To: netdev, John W. Linville, Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings, Gal Pressman
In-Reply-To: <1478100914-23610-1-git-send-email-galp@mellanox.com>

ethtool DEVNAME will now show actual port speed in addition
to physical port speed.

Signed-off-by: Gal Pressman <galp@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 27ff401..933b687 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -1504,9 +1504,16 @@ static int mlx5e_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
 	return err;
 }
 
+static int mlx5e_get_actual_speed(struct net_device *netdev)
+{
+	/* TODO: FW command to query the actual speed */
+	return SPEED_25000;
+}
+
 const struct ethtool_ops mlx5e_ethtool_ops = {
 	.get_drvinfo       = mlx5e_get_drvinfo,
 	.get_link          = ethtool_op_get_link,
+	.get_actual_speed  = mlx5e_get_actual_speed,
 	.get_strings       = mlx5e_get_strings,
 	.get_sset_count    = mlx5e_get_sset_count,
 	.get_ethtool_stats = mlx5e_get_ethtool_stats,
-- 
2.7.4

^ permalink raw reply related

* [PATCH RFC 1/2] ethtool: Add get actual port speed
From: Gal Pressman @ 2016-11-02 15:35 UTC (permalink / raw)
  To: netdev, John W. Linville, Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings, Gal Pressman
In-Reply-To: <1478100914-23610-1-git-send-email-galp@mellanox.com>

Add an additional actual speed field when using ethtool DEVNAME.
Actual speed will show the actual bandwidth exposed for the machine,
which may be different from the HCA operating speed.

Signed-off-by: Gal Pressman <galp@mellanox.com>
---
 include/linux/ethtool.h      |  1 +
 include/uapi/linux/ethtool.h |  2 ++
 net/core/ethtool.c           | 20 ++++++++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9ded8c6..215baa1 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -311,6 +311,7 @@ struct ethtool_ops {
 	void	(*set_msglevel)(struct net_device *, u32);
 	int	(*nway_reset)(struct net_device *);
 	u32	(*get_link)(struct net_device *);
+	int	(*get_actual_speed)(struct net_device *);
 	int	(*get_eeprom_len)(struct net_device *);
 	int	(*get_eeprom)(struct net_device *,
 			      struct ethtool_eeprom *, u8 *);
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 099a420..63057a2 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1315,6 +1315,8 @@ struct ethtool_per_queue_op {
 #define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
 
+#define ETHTOOL_GASPD		0x0000004e /* Get port actual speed */
+
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..a1921fd 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1516,6 +1516,22 @@ static int ethtool_get_link(struct net_device *dev, char __user *useraddr)
 	return 0;
 }
 
+static int ethtool_get_actual_speed(struct net_device *dev,
+				    char __user *useraddr)
+{
+	struct ethtool_value edata = { .cmd = ETHTOOL_GASPD };
+
+	if (!dev->ethtool_ops->get_actual_speed)
+		return -EOPNOTSUPP;
+
+	edata.data = dev->ethtool_ops->get_actual_speed(dev);
+
+	if (copy_to_user(useraddr, &edata, sizeof(edata)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int ethtool_get_any_eeprom(struct net_device *dev, void __user *useraddr,
 				  int (*getter)(struct net_device *,
 						struct ethtool_eeprom *, u8 *),
@@ -2450,6 +2466,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GDRVINFO:
 	case ETHTOOL_GMSGLVL:
 	case ETHTOOL_GLINK:
+	case ETHTOOL_GASPD:
 	case ETHTOOL_GCOALESCE:
 	case ETHTOOL_GRINGPARAM:
 	case ETHTOOL_GPAUSEPARAM:
@@ -2531,6 +2548,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GLINK:
 		rc = ethtool_get_link(dev, useraddr);
 		break;
+	case ETHTOOL_GASPD:
+		rc = ethtool_get_actual_speed(dev, useraddr);
+		break;
 	case ETHTOOL_GEEPROM:
 		rc = ethtool_get_eeprom(dev, useraddr);
 		break;
-- 
2.7.4

^ permalink raw reply related

* [PATCH RFC 0/2] ethtool: Add actual port speed reporting
From: Gal Pressman @ 2016-11-02 15:35 UTC (permalink / raw)
  To: netdev, John W. Linville, Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings, Gal Pressman

Sending RFC to get feedback for the following ethtool proposal:

In some cases such as virtual machines and multi functions (SR-IOV), the actual
bandwidth exposed for each machine is not accurately shown in ethtool.
Currently ethtool shows only physical port link speed.
In our case we would like to show the virtual port operational link speed
which in some cases is less than the physical port speed.

This will give users better visibility for the actual speed running on their device.

$ ethtool ens6
...
Speed: 50000Mb/s
Actual speed: 25000Mb/s

Gal Pressman (2):
  ethtool: Add get actual port speed support
  net/mlx5e: Add support for ethtool get actual speed callback

 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c |  7 +++++++
 include/linux/ethtool.h                              |  1 +
 include/uapi/linux/ethtool.h                         |  2 ++
 net/core/ethtool.c                                   | 20 ++++++++++++++++++++
 4 files changed, 30 insertions(+)

-- 
2.7.4

^ permalink raw reply

* Re: [patch net-next 0/2] Fixes for raw diag sockets handling
From: Cyrill Gorcunov @ 2016-11-02 15:29 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev, Eric Dumazet, David S. Miller, Andrey Vagin,
	Stephen Hemminger
In-Reply-To: <2a540f59-4495-573c-7dda-0303f1810e6e@cumulusnetworks.com>

On Wed, Nov 02, 2016 at 09:10:32AM -0600, David Ahern wrote:
> > @__dif != 0 the match may return socket where sk_bound_dev_if = 0
> > instead of completely matching one. Isn't it?
> 
> yes. I recently added an exact_dif to the lookup for listener sockets
> (see compute_score). Something like that could be added to INET_MATCH.

Seem so. I need to revisit this moment. Because with current lookup code
iproute2 patches I made and been testing do not kill all sockets bound
to particular device in one pass (because request from userspace asks
for index 15 in my case but kernel return one with index 0). At first
I thought I made a mistake in userspace code but once I added printk's
into kernel I found that here some strange results over lookup.

	Cyrill

^ permalink raw reply

* Re: Let's do P4
From: Jiri Pirko @ 2016-11-02 15:27 UTC (permalink / raw)
  To: John Fastabend
  Cc: Daniel Borkmann, Alexei Starovoitov, Thomas Graf, Jakub Kicinski,
	netdev, davem, jhs, roopa, simon.horman, ast, prem, hannes, jbenc,
	tom, mattyk, idosch, eladr, yotamg, nogahf, ogerlitz, linville,
	andy, f.fainelli, dsa, vivien.didelot, andrew, ivecera,
	Maciej Żenczykowski
In-Reply-To: <581A04CA.5020808@gmail.com>

Wed, Nov 02, 2016 at 04:22:50PM CET, john.fastabend@gmail.com wrote:

[...]

>>>
>>> What is your compilerA? Is that part of tc in user space? Maybe linked
>> 
>> It is something that transforms original p4 source to some intermediate
>> form, easy to be processed by in-kernel compilers.
>> 
>> 
>>> against LLVM lib, for example? If you really want some sw path, can't tc
>>> do this transparently from user space instead when it gets a netlink error
>>> that it cannot get offloaded (and thus switch internally to f_bpf's loader)?
>> 
>> In real life, user will most probably use p4 for hw programming, but the
>> sw fallback will be done in bpf directly. In that case, he would use
>> cls_bfp SKIP_HW
>> cls_p4 SKIP_SW
>> 
>> But in order to allow cls_p4 offloading to hw, we need in-kernel
>> interpreter. That is purpose of compilerB to take agvantage of bpf, but
>> the in-kernel interpreter could be implemented differently.
>> 
>
>But this is the issue. We openly acknowledge it wont actually be used.
>We have multiple user space compilers that generate at least half way
>reasonable ebpf code that is being used in real deployments and
>works great for testing. This looks like pure overhead to satisfy this
>hw/sw parity checkbox and I can't see why anyone would use it or even
>maintain it. Looks like a checkbox and I like to avoid useless work that
>is likely to bit rot.

That's how it works I'm afraid, unless something changed from the last
time we discussed this. Without in-kernel implementation, it's a bypass.

Dave?

^ permalink raw reply

* Re: [PATCH net-next v2] ipv4: fib: Replay events when registering FIB notifier
From: David Miller @ 2016-11-02 15:26 UTC (permalink / raw)
  To: jiri
  Cc: roopa, eric.dumazet, idosch, netdev, jiri, mlxsw, dsa, nikolay,
	andy, vivien.didelot, andrew, f.fainelli, alexander.h.duyck,
	kuznet, jmorris, yoshfuji, kaber, idosch
In-Reply-To: <20161102073502.GB1713@nanopsycho.orion>

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 2 Nov 2016 08:35:02 +0100

> How do you imagine this mode should looks like? Could you draw me some
> example?

Well, first of all, there is no reason we can't provide a mechanism by
which the driver can request and obtain a FIB dump.

And it can be designed in a way to be preemptible or at least not
require RTNL to be held during the entire operation.  Sequence
counters or similar can be used to make sure that if the table changes
mid-dump due to RTNL being dropped, the dump can be rewound and
restarted.

^ permalink raw reply

* Re: Let's do P4
From: Jiri Pirko @ 2016-11-02 15:23 UTC (permalink / raw)
  To: John Fastabend
  Cc: Alexei Starovoitov, Thomas Graf, Jakub Kicinski, netdev, davem,
	jhs, roopa, simon.horman, ast, daniel, prem, hannes, jbenc, tom,
	mattyk, idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
	f.fainelli, dsa, vivien.didelot, andrew, ivecera,
	Maciej Żenczykowski
In-Reply-To: <581A03AE.8050006@gmail.com>

Wed, Nov 02, 2016 at 04:18:06PM CET, john.fastabend@gmail.com wrote:
>On 16-11-02 01:07 AM, Jiri Pirko wrote:
>> Tue, Nov 01, 2016 at 04:13:32PM CET, john.fastabend@gmail.com wrote:

[...]


>[...]>
>>>
>>> Same question as above are we _really_ talking about pushing the entire
>>> programmability of the device via 'tc'. If so we need to have a vendor
>>> say they will support and implement this?
>> 
>> We need some API, and I believe that TC is perfectly suitable for that.
>> Why do you think it's a problem?
>> 
>
>For runtime configuration completely agree. For device configuration
>I don't see the advantage of adding an entire device specific compiler
>in the driver. The device is a set of CAMs, TCAMs, ALUs, instruction
>caches, etc. its not like a typical NIC/switch where you just bang
>some registers. Unless its all done in firmware but that creates an
>entirely different set of problems like how to update your compiler.
>
>Bottom line we need to have a proof point of a driver in kernel
>to see exactly how a P4 configuration would work. Again runtime config
>and device topology/capabilities discovery I'm completely on board.

I think we need to implement P4 world in rocker. Any volunteer? :)

^ permalink raw reply

* Re: Let's do P4
From: John Fastabend @ 2016-11-02 15:22 UTC (permalink / raw)
  To: Jiri Pirko, Daniel Borkmann
  Cc: Alexei Starovoitov, Thomas Graf, Jakub Kicinski, netdev, davem,
	jhs, roopa, simon.horman, ast, prem, hannes, jbenc, tom, mattyk,
	idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
	f.fainelli, dsa, vivien.didelot, andrew, ivecera,
	Maciej Żenczykowski
In-Reply-To: <20161102081404.GE1713@nanopsycho.orion>

[...]

>>>>> Exactly. Following drawing shows p4 pipeline setup for SW and Hw:
>>>>>
>>>>>                                   |
>>>>>                                   |               +--> ebpf engine
>>>>>                                   |               |
>>>>>                                   |               |
>>>>>                                   |           compilerB
>>>>>                                   |               ^
>>>>>                                   |               |
>>>>> p4src --> compilerA --> p4ast --TCNL--> cls_p4 --+-> driver -> compilerC -> HW
>>>>>                                   |
>>>>>                         userspace | kernel
>>>>>                                   |
>>
>> Sorry for jumping into the middle and the delay (plumbers this week). My
>> question would be, if the main target is for p4 *offloading* anyway, who
>> would use this sw fallback path? Mostly for testing purposes?
> 
> Development and testing purposes, yes.
> 
> 
>>
>> I'm not sure about compilerB here and the complexity that needs to be
>> pushed into the kernel along with it. I would assume this would result
>> in slower code than what the existing P4 -> eBPF front ends for LLVM
>> would generate since it could perform all kind of optimizations there,
> 
> The complexity would be similar to compilerC. For compilerB,
> optimizations does not really matter, as it it for testing mainly.
> 
> 
>> that might not be feasible for doing inside the kernel. Thus, if I'd want
>> to do that in sw, I'd just use the existing LLVM facilities instead and
>> go via cls_bpf in that case.
>>
>> What is your compilerA? Is that part of tc in user space? Maybe linked
> 
> It is something that transforms original p4 source to some intermediate
> form, easy to be processed by in-kernel compilers.
> 
> 
>> against LLVM lib, for example? If you really want some sw path, can't tc
>> do this transparently from user space instead when it gets a netlink error
>> that it cannot get offloaded (and thus switch internally to f_bpf's loader)?
> 
> In real life, user will most probably use p4 for hw programming, but the
> sw fallback will be done in bpf directly. In that case, he would use
> cls_bfp SKIP_HW
> cls_p4 SKIP_SW
> 
> But in order to allow cls_p4 offloading to hw, we need in-kernel
> interpreter. That is purpose of compilerB to take agvantage of bpf, but
> the in-kernel interpreter could be implemented differently.
> 

But this is the issue. We openly acknowledge it wont actually be used.
We have multiple user space compilers that generate at least half way
reasonable ebpf code that is being used in real deployments and
works great for testing. This looks like pure overhead to satisfy this
hw/sw parity checkbox and I can't see why anyone would use it or even
maintain it. Looks like a checkbox and I like to avoid useless work that
is likely to bit rot.

^ permalink raw reply

* Re: Let's do P4
From: John Fastabend @ 2016-11-02 15:18 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Alexei Starovoitov, Thomas Graf, Jakub Kicinski, netdev, davem,
	jhs, roopa, simon.horman, ast, daniel, prem, hannes, jbenc, tom,
	mattyk, idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
	f.fainelli, dsa, vivien.didelot, andrew, ivecera,
	Maciej Żenczykowski
In-Reply-To: <20161102080723.GD1713@nanopsycho.orion>

On 16-11-02 01:07 AM, Jiri Pirko wrote:
> Tue, Nov 01, 2016 at 04:13:32PM CET, john.fastabend@gmail.com wrote:
>> [...]
>>
>>>>> P4 is ment to program programable hw, not fixed pipeline.
>>>>>
>>>>
>>>> I'm guessing there are no upstream drivers at the moment that support
>>>> this though right? The rocker universe bits though could leverage this.
>>>
>>> mlxsw. But this is naturaly not implemented yet, as there is no
>>> infrastructure.
>>
>> Really? What is re-programmable?
>>
>> Can the parse graph support arbitrary parse graph?
>> Can the table topology be reconfigured?
>> Can new tables be created?
>> What about "new" actions being defined at configuration time?
>>
>> Or is this just the normal TCAM configuration of defining key widths and
>> fields.
> 
> At this point TCAM configuration.
> 

OK so before we go down the path to enable a full fledged P4 interface
we need a consumer for sure. We shouldn't add all this complexity until
someone steps up to use it. A runtime API is sufficient for TCAM config.

[...]

>>
>> P4-16 will allow externs, "functions" to execute in the control flow and
>> possibly inside the parse graph. None of this was considered in the
>> Flow-API. So none of this is supported.
>>
>> I still have the question are you trying to push the "programming" of
>> the device via 'tc' or just the runtime configuration of tables? If it
>> is just runtime Flow-API is sufficient IMO. If its programming the
>> device using the complete P4-16 spec than no its not sufficient. But
> 
> Sure we need both.
> 

See above.

> 
>> I don't believe vendors will expose the complete programmability of the
>> device in the driver, this is going to look more like a fw update than
>> a runtime change at least on the devices I'm aware of.
> 
> Depends on driver. I think it is fine if driver processed it into come
> hw configuration sequence or it simply pushed the program down to fw.
> Both usecases are legit.
> 

At this point I don't think the entire P4 capabilities will be exposed
as an API but more along the lines of an FPGA bitstream or firmware
update.


[...]

>>
>> Same question as above are we _really_ talking about pushing the entire
>> programmability of the device via 'tc'. If so we need to have a vendor
>> say they will support and implement this?
> 
> We need some API, and I believe that TC is perfectly suitable for that.
> Why do you think it's a problem?
> 

For runtime configuration completely agree. For device configuration
I don't see the advantage of adding an entire device specific compiler
in the driver. The device is a set of CAMs, TCAMs, ALUs, instruction
caches, etc. its not like a typical NIC/switch where you just bang
some registers. Unless its all done in firmware but that creates an
entirely different set of problems like how to update your compiler.

Bottom line we need to have a proof point of a driver in kernel
to see exactly how a P4 configuration would work. Again runtime config
and device topology/capabilities discovery I'm completely on board.

Thanks,
John

^ permalink raw reply

* [PATCH net-next V3 0/3] mlx4 XDP TX refactor
From: Tariq Toukan @ 2016-11-02 15:12 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Brenden Blanco, Alexei Starovoitov,
	Saeed Mahameed, Tariq Toukan

Hi Dave,

This patchset refactors the XDP forwarding case, so that
its dedicated transmit queues are managed in a complete
separation from the other regular ones.

It also adds ethtool counters for XDP cases.

Series generated against net-next commit:
22ca904ad70a genetlink: fix error return code in genl_register_family()

Thanks,
Tariq.

v3:
* Exposed per ring counters.

v2:
* Added ethtool counters.
* Rebased, now patch 2 reverts Brenden's fix, as the bug no longer exists:
  958b3d396d7f ("net/mlx4_en: fixup xdp tx irq to match rx")
* Updated commit message of patch 2.

Tariq Toukan (3):
  net/mlx4_en: Add TX_XDP for CQ types
  net/mlx4_en: Refactor the XDP forwarding rings scheme
  net/mlx4_en: Add ethtool statistics for XDP cases

 drivers/net/ethernet/mellanox/mlx4/en_cq.c      |  28 +-
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 101 +++++--
 drivers/net/ethernet/mellanox/mlx4/en_main.c    |   2 +-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c  | 382 +++++++++++++++---------
 drivers/net/ethernet/mellanox/mlx4/en_port.c    |  10 +-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c      |  20 +-
 drivers/net/ethernet/mellanox/mlx4/en_tx.c      |  17 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |  25 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h |  10 +-
 9 files changed, 366 insertions(+), 229 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH net-next V3 3/3] net/mlx4_en: Add ethtool statistics for XDP cases
From: Tariq Toukan @ 2016-11-02 15:12 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Brenden Blanco, Alexei Starovoitov,
	Saeed Mahameed, Tariq Toukan
In-Reply-To: <1478099545-24181-1-git-send-email-tariqt@mellanox.com>

XDP statistics are reported in ethtool, in total and per ring,
as follows:
- xdp_drop: the number of packets dropped by xdp.
- xdp_tx: the number of packets forwarded by xdp.
- xdp_tx_full: the number of times an xdp forward failed
	due to a full tx xdp ring.

In addition, all packets that are dropped/forwarded by XDP
are no longer accounted in rx_packets/rx_bytes of the ring,
so that they count traffic that is passed to the stack.

Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 25 ++++++++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c  |  4 ++++
 drivers/net/ethernet/mellanox/mlx4/en_port.c    |  6 ++++++
 drivers/net/ethernet/mellanox/mlx4/en_rx.c      | 12 +++++++-----
 drivers/net/ethernet/mellanox/mlx4/en_tx.c      |  8 ++++----
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |  7 ++++++-
 drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h | 10 +++++++++-
 7 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index e8ccb95680bc..487a58f9c192 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -195,6 +195,10 @@ static int mlx4_en_moderation_update(struct mlx4_en_priv *priv)
 	"tx_prio_7_packets", "tx_prio_7_bytes",
 	"tx_novlan_packets", "tx_novlan_bytes",
 
+	/* xdp statistics */
+	"rx_xdp_drop",
+	"rx_xdp_tx",
+	"rx_xdp_tx_full",
 };
 
 static const char mlx4_en_test_names[][ETH_GSTRING_LEN]= {
@@ -340,7 +344,7 @@ static int mlx4_en_get_sset_count(struct net_device *dev, int sset)
 	case ETH_SS_STATS:
 		return bitmap_iterator_count(&it) +
 			(priv->tx_ring_num[TX] * 2) +
-			(priv->rx_ring_num * 3);
+			(priv->rx_ring_num * (3 + NUM_XDP_STATS));
 	case ETH_SS_TEST:
 		return MLX4_EN_NUM_SELF_TEST - !(priv->mdev->dev->caps.flags
 					& MLX4_DEV_CAP_FLAG_UC_LOOPBACK) * 2;
@@ -400,6 +404,10 @@ static void mlx4_en_get_ethtool_stats(struct net_device *dev,
 		if (bitmap_iterator_test(&it))
 			data[index++] = ((unsigned long *)&priv->pkstats)[i];
 
+	for (i = 0; i < NUM_XDP_STATS; i++, bitmap_iterator_inc(&it))
+		if (bitmap_iterator_test(&it))
+			data[index++] = ((unsigned long *)&priv->xdp_stats)[i];
+
 	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
 		data[index++] = priv->tx_ring[TX][i]->packets;
 		data[index++] = priv->tx_ring[TX][i]->bytes;
@@ -408,6 +416,9 @@ static void mlx4_en_get_ethtool_stats(struct net_device *dev,
 		data[index++] = priv->rx_ring[i]->packets;
 		data[index++] = priv->rx_ring[i]->bytes;
 		data[index++] = priv->rx_ring[i]->dropped;
+		data[index++] = priv->rx_ring[i]->xdp_drop;
+		data[index++] = priv->rx_ring[i]->xdp_tx;
+		data[index++] = priv->rx_ring[i]->xdp_tx_full;
 	}
 	spin_unlock_bh(&priv->stats_lock);
 
@@ -470,6 +481,12 @@ static void mlx4_en_get_strings(struct net_device *dev,
 				strcpy(data + (index++) * ETH_GSTRING_LEN,
 				       main_strings[strings]);
 
+		for (i = 0; i < NUM_XDP_STATS; i++, strings++,
+		     bitmap_iterator_inc(&it))
+			if (bitmap_iterator_test(&it))
+				strcpy(data + (index++) * ETH_GSTRING_LEN,
+				       main_strings[strings]);
+
 		for (i = 0; i < priv->tx_ring_num[TX]; i++) {
 			sprintf(data + (index++) * ETH_GSTRING_LEN,
 				"tx%d_packets", i);
@@ -483,6 +500,12 @@ static void mlx4_en_get_strings(struct net_device *dev,
 				"rx%d_bytes", i);
 			sprintf(data + (index++) * ETH_GSTRING_LEN,
 				"rx%d_dropped", i);
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_xdp_drop", i);
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_xdp_tx", i);
+			sprintf(data + (index++) * ETH_GSTRING_LEN,
+				"rx%d_xdp_tx_full", i);
 		}
 		break;
 	case ETH_SS_PRIV_FLAGS:
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index edf0a99177e1..0f6225c042be 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3125,6 +3125,10 @@ void mlx4_en_set_stats_bitmap(struct mlx4_dev *dev,
 
 	if (!mlx4_is_slave(dev))
 		bitmap_set(stats_bitmap->bitmap, last_i, NUM_PKT_STATS);
+	last_i += NUM_PKT_STATS;
+
+	bitmap_set(stats_bitmap->bitmap, last_i, NUM_XDP_STATS);
+	last_i += NUM_XDP_STATS;
 }
 
 int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.c b/drivers/net/ethernet/mellanox/mlx4/en_port.c
index 2e1ab4642569..1eb4c1e10bad 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_port.c
@@ -179,6 +179,9 @@ int mlx4_en_DUMP_ETH_STATS(struct mlx4_en_dev *mdev, u8 port, u8 reset)
 	priv->port_stats.rx_chksum_good = 0;
 	priv->port_stats.rx_chksum_none = 0;
 	priv->port_stats.rx_chksum_complete = 0;
+	priv->xdp_stats.rx_xdp_drop    = 0;
+	priv->xdp_stats.rx_xdp_tx      = 0;
+	priv->xdp_stats.rx_xdp_tx_full = 0;
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		stats->rx_packets += priv->rx_ring[i]->packets;
 		stats->rx_bytes += priv->rx_ring[i]->bytes;
@@ -186,6 +189,9 @@ int mlx4_en_DUMP_ETH_STATS(struct mlx4_en_dev *mdev, u8 port, u8 reset)
 		priv->port_stats.rx_chksum_good += priv->rx_ring[i]->csum_ok;
 		priv->port_stats.rx_chksum_none += priv->rx_ring[i]->csum_none;
 		priv->port_stats.rx_chksum_complete += priv->rx_ring[i]->csum_complete;
+		priv->xdp_stats.rx_xdp_drop    += priv->rx_ring[i]->xdp_drop;
+		priv->xdp_stats.rx_xdp_tx      += priv->rx_ring[i]->xdp_tx;
+		priv->xdp_stats.rx_xdp_tx_full += priv->rx_ring[i]->xdp_tx_full;
 	}
 	stats->tx_packets = 0;
 	stats->tx_bytes = 0;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 71196f68b55d..2cc91002064f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -875,8 +875,6 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 		 */
 		length = be32_to_cpu(cqe->byte_cnt);
 		length -= ring->fcs_del;
-		ring->bytes += length;
-		ring->packets++;
 		l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) &&
 			(cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL));
 
@@ -902,22 +900,26 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 			case XDP_PASS:
 				break;
 			case XDP_TX:
-				if (likely(!mlx4_en_xmit_frame(frags, dev,
+				if (likely(!mlx4_en_xmit_frame(ring, frags, dev,
 							length, cq->ring,
 							&doorbell_pending)))
 					goto consumed;
-				goto xdp_drop; /* Drop on xmit failure */
+				goto xdp_drop_no_cnt; /* Drop on xmit failure */
 			default:
 				bpf_warn_invalid_xdp_action(act);
 			case XDP_ABORTED:
 			case XDP_DROP:
-xdp_drop:
+				ring->xdp_drop++;
+xdp_drop_no_cnt:
 				if (likely(mlx4_en_rx_recycle(ring, frags)))
 					goto consumed;
 				goto next;
 			}
 		}
 
+		ring->bytes += length;
+		ring->packets++;
+
 		if (likely(dev->features & NETIF_F_RXCSUM)) {
 			if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP |
 						      MLX4_CQE_STATUS_UDP)) {
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 95dc864bb2f7..5de3cbe24f2b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -1079,7 +1079,8 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
-netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_alloc *frame,
+netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_ring *rx_ring,
+			       struct mlx4_en_rx_alloc *frame,
 			       struct net_device *dev, unsigned int length,
 			       int tx_ind, int *doorbell_pending)
 {
@@ -1154,8 +1155,7 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_alloc *frame,
 		((ring->prod & ring->size) ?
 		 cpu_to_be32(MLX4_EN_BIT_DESC_OWN) : 0);
 
-	ring->packets++;
-	ring->bytes += tx_info->nr_bytes;
+	rx_ring->xdp_tx++;
 	AVG_PERF_COUNTER(priv->pstats.tx_pktsz_avg, length);
 
 	ring->prod += nr_txbb;
@@ -1179,7 +1179,7 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_alloc *frame,
 	return NETDEV_TX_OK;
 
 tx_drop_count:
-	ring->tx_dropped++;
+	rx_ring->xdp_tx_full++;
 tx_drop:
 	return NETDEV_TX_BUSY;
 }
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 6e0693659f85..eff21651b673 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -350,6 +350,9 @@ struct mlx4_en_rx_ring {
 	unsigned long csum_ok;
 	unsigned long csum_none;
 	unsigned long csum_complete;
+	unsigned long xdp_drop;
+	unsigned long xdp_tx;
+	unsigned long xdp_tx_full;
 	unsigned long dropped;
 	int hwtstamp_rx_filter;
 	cpumask_var_t affinity_mask;
@@ -599,6 +602,7 @@ struct mlx4_en_priv {
 	struct mlx4_en_flow_stats_rx rx_flowstats;
 	struct mlx4_en_flow_stats_tx tx_flowstats;
 	struct mlx4_en_port_stats port_stats;
+	struct mlx4_en_xdp_stats xdp_stats;
 	struct mlx4_en_stats_bitmap stats_bitmap;
 	struct list_head mc_list;
 	struct list_head curr_list;
@@ -687,7 +691,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb,
 			 void *accel_priv, select_queue_fallback_t fallback);
 netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev);
-netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_alloc *frame,
+netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_ring *rx_ring,
+			       struct mlx4_en_rx_alloc *frame,
 			       struct net_device *dev, unsigned int length,
 			       int tx_ind, int *doorbell_pending);
 void mlx4_en_xmit_doorbell(struct mlx4_en_tx_ring *ring);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h
index 7fd466c0b929..48641cb0367f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_stats.h
@@ -55,6 +55,13 @@ struct mlx4_en_perf_stats {
 #define NUM_PERF_COUNTERS		6
 };
 
+struct mlx4_en_xdp_stats {
+	unsigned long rx_xdp_drop;
+	unsigned long rx_xdp_tx;
+	unsigned long rx_xdp_tx_full;
+#define NUM_XDP_STATS		3
+};
+
 #define NUM_MAIN_STATS	21
 
 #define MLX4_NUM_PRIORITIES	8
@@ -107,7 +114,8 @@ enum {
 };
 
 #define NUM_ALL_STATS	(NUM_MAIN_STATS + NUM_PORT_STATS + NUM_PKT_STATS + \
-			 NUM_FLOW_STATS + NUM_PERF_STATS + NUM_PF_STATS)
+			 NUM_FLOW_STATS + NUM_PERF_STATS + NUM_PF_STATS + \
+			 NUM_XDP_STATS)
 
 #define MLX4_FIND_NETDEV_STAT(n) (offsetof(struct net_device_stats, n) / \
 				  sizeof(((struct net_device_stats *)0)->n))
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next V3 2/3] net/mlx4_en: Refactor the XDP forwarding rings scheme
From: Tariq Toukan @ 2016-11-02 15:12 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Brenden Blanco, Alexei Starovoitov,
	Saeed Mahameed, Tariq Toukan
In-Reply-To: <1478099545-24181-1-git-send-email-tariqt@mellanox.com>

Separately manage the two types of TX rings: regular ones, and XDP.
Upon an XDP set, do not borrow regular TX rings and convert them
into XDP ones, but allocate new ones, unless we hit the max number
of rings.
Which means that in systems with smaller #cores we will not consume
the current TX rings for XDP, while we are still in the num TX limit.

XDP TX rings counters are not shown in ethtool statistics.
Instead, XDP counters will be added to the respective RX rings
in a downstream patch.

This has no performance implications.

Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_cq.c      |  10 +-
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  76 +++--
 drivers/net/ethernet/mellanox/mlx4/en_main.c    |   2 +-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c  | 378 ++++++++++++++----------
 drivers/net/ethernet/mellanox/mlx4/en_port.c    |   4 +-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c      |   8 +-
 drivers/net/ethernet/mellanox/mlx4/en_tx.c      |   9 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |  11 +-
 8 files changed, 293 insertions(+), 205 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 1427311a9640..03f05c4d1f98 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -127,15 +127,7 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 		/* For TX we use the same irq per
 		ring we assigned for the RX    */
 		struct mlx4_en_cq *rx_cq;
-		int xdp_index;
-
-		/* The xdp tx irq must align with the rx ring that forwards to
-		 * it, so reindex these from 0. This should only happen when
-		 * tx_ring_num is not a multiple of rx_ring_num.
-		 */
-		xdp_index = (priv->xdp_ring_num - priv->tx_ring_num) + cq_idx;
-		if (xdp_index >= 0)
-			cq_idx = xdp_index;
+
 		cq_idx = cq_idx % priv->rx_ring_num;
 		rx_cq = priv->rx_cq[cq_idx];
 		cq->vector = rx_cq->vector;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index bdda17d2ea0f..e8ccb95680bc 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -49,16 +49,19 @@
 
 static int mlx4_en_moderation_update(struct mlx4_en_priv *priv)
 {
-	int i;
+	int i, t;
 	int err = 0;
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		priv->tx_cq[i]->moder_cnt = priv->tx_frames;
-		priv->tx_cq[i]->moder_time = priv->tx_usecs;
-		if (priv->port_up) {
-			err = mlx4_en_set_cq_moder(priv, priv->tx_cq[i]);
-			if (err)
-				return err;
+	for (t = 0 ; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			priv->tx_cq[t][i]->moder_cnt = priv->tx_frames;
+			priv->tx_cq[t][i]->moder_time = priv->tx_usecs;
+			if (priv->port_up) {
+				err = mlx4_en_set_cq_moder(priv,
+							   priv->tx_cq[t][i]);
+				if (err)
+					return err;
+			}
 		}
 	}
 
@@ -336,7 +339,7 @@ static int mlx4_en_get_sset_count(struct net_device *dev, int sset)
 	switch (sset) {
 	case ETH_SS_STATS:
 		return bitmap_iterator_count(&it) +
-			(priv->tx_ring_num * 2) +
+			(priv->tx_ring_num[TX] * 2) +
 			(priv->rx_ring_num * 3);
 	case ETH_SS_TEST:
 		return MLX4_EN_NUM_SELF_TEST - !(priv->mdev->dev->caps.flags
@@ -397,9 +400,9 @@ static void mlx4_en_get_ethtool_stats(struct net_device *dev,
 		if (bitmap_iterator_test(&it))
 			data[index++] = ((unsigned long *)&priv->pkstats)[i];
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		data[index++] = priv->tx_ring[i]->packets;
-		data[index++] = priv->tx_ring[i]->bytes;
+	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
+		data[index++] = priv->tx_ring[TX][i]->packets;
+		data[index++] = priv->tx_ring[TX][i]->bytes;
 	}
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		data[index++] = priv->rx_ring[i]->packets;
@@ -467,7 +470,7 @@ static void mlx4_en_get_strings(struct net_device *dev,
 				strcpy(data + (index++) * ETH_GSTRING_LEN,
 				       main_strings[strings]);
 
-		for (i = 0; i < priv->tx_ring_num; i++) {
+		for (i = 0; i < priv->tx_ring_num[TX]; i++) {
 			sprintf(data + (index++) * ETH_GSTRING_LEN,
 				"tx%d_packets", i);
 			sprintf(data + (index++) * ETH_GSTRING_LEN,
@@ -1060,7 +1063,7 @@ static int mlx4_en_set_ringparam(struct net_device *dev,
 
 	if (rx_size == (priv->port_up ? priv->rx_ring[0]->actual_size :
 					priv->rx_ring[0]->size) &&
-	    tx_size == priv->tx_ring[0]->size)
+	    tx_size == priv->tx_ring[TX][0]->size)
 		return 0;
 
 	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
@@ -1105,7 +1108,7 @@ static void mlx4_en_get_ringparam(struct net_device *dev,
 	param->tx_max_pending = MLX4_EN_MAX_TX_SIZE;
 	param->rx_pending = priv->port_up ?
 		priv->rx_ring[0]->actual_size : priv->rx_ring[0]->size;
-	param->tx_pending = priv->tx_ring[0]->size;
+	param->tx_pending = priv->tx_ring[TX][0]->size;
 }
 
 static u32 mlx4_en_get_rxfh_indir_size(struct net_device *dev)
@@ -1710,7 +1713,7 @@ static void mlx4_en_get_channels(struct net_device *dev,
 	channel->max_tx = MLX4_EN_MAX_TX_RING_P_UP;
 
 	channel->rx_count = priv->rx_ring_num;
-	channel->tx_count = priv->tx_ring_num / MLX4_EN_NUM_UP;
+	channel->tx_count = priv->tx_ring_num[TX] / MLX4_EN_NUM_UP;
 }
 
 static int mlx4_en_set_channels(struct net_device *dev,
@@ -1721,6 +1724,7 @@ static int mlx4_en_set_channels(struct net_device *dev,
 	struct mlx4_en_port_profile new_prof;
 	struct mlx4_en_priv *tmp;
 	int port_up = 0;
+	int xdp_count;
 	int err = 0;
 
 	if (channel->other_count || channel->combined_count ||
@@ -1729,20 +1733,25 @@ static int mlx4_en_set_channels(struct net_device *dev,
 	    !channel->tx_count || !channel->rx_count)
 		return -EINVAL;
 
-	if (channel->tx_count * MLX4_EN_NUM_UP <= priv->xdp_ring_num) {
-		en_err(priv, "Minimum %d tx channels required with XDP on\n",
-		       priv->xdp_ring_num / MLX4_EN_NUM_UP + 1);
-		return -EINVAL;
-	}
-
 	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
 	if (!tmp)
 		return -ENOMEM;
 
 	mutex_lock(&mdev->state_lock);
+	xdp_count = priv->tx_ring_num[TX_XDP] ? channel->rx_count : 0;
+	if (channel->tx_count * MLX4_EN_NUM_UP + xdp_count > MAX_TX_RINGS) {
+		err = -EINVAL;
+		en_err(priv,
+		       "Total number of TX and XDP rings (%d) exceeds the maximum supported (%d)\n",
+		       channel->tx_count * MLX4_EN_NUM_UP + xdp_count,
+		       MAX_TX_RINGS);
+		goto out;
+	}
+
 	memcpy(&new_prof, priv->prof, sizeof(struct mlx4_en_port_profile));
 	new_prof.num_tx_rings_p_up = channel->tx_count;
-	new_prof.tx_ring_num = channel->tx_count * MLX4_EN_NUM_UP;
+	new_prof.tx_ring_num[TX] = channel->tx_count * MLX4_EN_NUM_UP;
+	new_prof.tx_ring_num[TX_XDP] = xdp_count;
 	new_prof.rx_ring_num = channel->rx_count;
 
 	err = mlx4_en_try_alloc_resources(priv, tmp, &new_prof);
@@ -1756,14 +1765,13 @@ static int mlx4_en_set_channels(struct net_device *dev,
 
 	mlx4_en_safe_replace_resources(priv, tmp);
 
-	netif_set_real_num_tx_queues(dev, priv->tx_ring_num -
-							priv->xdp_ring_num);
+	netif_set_real_num_tx_queues(dev, priv->tx_ring_num[TX]);
 	netif_set_real_num_rx_queues(dev, priv->rx_ring_num);
 
 	if (dev->num_tc)
 		mlx4_en_setup_tc(dev, MLX4_EN_NUM_UP);
 
-	en_warn(priv, "Using %d TX rings\n", priv->tx_ring_num);
+	en_warn(priv, "Using %d TX rings\n", priv->tx_ring_num[TX]);
 	en_warn(priv, "Using %d RX rings\n", priv->rx_ring_num);
 
 	if (port_up) {
@@ -1774,8 +1782,8 @@ static int mlx4_en_set_channels(struct net_device *dev,
 
 	err = mlx4_en_moderation_update(priv);
 out:
-	kfree(tmp);
 	mutex_unlock(&mdev->state_lock);
+	kfree(tmp);
 	return err;
 }
 
@@ -1823,11 +1831,15 @@ static int mlx4_en_set_priv_flags(struct net_device *dev, u32 flags)
 	int ret = 0;
 
 	if (bf_enabled_new != bf_enabled_old) {
+		int t;
+
 		if (bf_enabled_new) {
 			bool bf_supported = true;
 
-			for (i = 0; i < priv->tx_ring_num; i++)
-				bf_supported &= priv->tx_ring[i]->bf_alloced;
+			for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++)
+				for (i = 0; i < priv->tx_ring_num[t]; i++)
+					bf_supported &=
+						priv->tx_ring[t][i]->bf_alloced;
 
 			if (!bf_supported) {
 				en_err(priv, "BlueFlame is not supported\n");
@@ -1839,8 +1851,10 @@ static int mlx4_en_set_priv_flags(struct net_device *dev, u32 flags)
 			priv->pflags &= ~MLX4_EN_PRIV_FLAGS_BLUEFLAME;
 		}
 
-		for (i = 0; i < priv->tx_ring_num; i++)
-			priv->tx_ring[i]->bf_enabled = bf_enabled_new;
+		for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++)
+			for (i = 0; i < priv->tx_ring_num[t]; i++)
+				priv->tx_ring[t][i]->bf_enabled =
+					bf_enabled_new;
 
 		en_info(priv, "BlueFlame %s\n",
 			bf_enabled_new ?  "Enabled" : "Disabled");
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c
index bf7628db098a..36a7a54bbb82 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c
@@ -169,7 +169,7 @@ static int mlx4_en_get_profile(struct mlx4_en_dev *mdev)
 		params->prof[i].tx_ppp = pfctx;
 		params->prof[i].tx_ring_size = MLX4_EN_DEF_TX_RING_SIZE;
 		params->prof[i].rx_ring_size = MLX4_EN_DEF_RX_RING_SIZE;
-		params->prof[i].tx_ring_num = params->num_tx_rings_p_up *
+		params->prof[i].tx_ring_num[TX] = params->num_tx_rings_p_up *
 			MLX4_EN_NUM_UP;
 		params->prof[i].rss_rings = 0;
 		params->prof[i].inline_thold = inline_thold;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 58b749dd6125..edf0a99177e1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1214,8 +1214,8 @@ static void mlx4_en_netpoll(struct net_device *dev)
 	struct mlx4_en_cq *cq;
 	int i;
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		cq = priv->tx_cq[i];
+	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
+		cq = priv->tx_cq[TX][i];
 		napi_schedule(&cq->napi);
 	}
 }
@@ -1299,12 +1299,14 @@ static void mlx4_en_tx_timeout(struct net_device *dev)
 	if (netif_msg_timer(priv))
 		en_warn(priv, "Tx timeout called on port:%d\n", priv->port);
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
+	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
+		struct mlx4_en_tx_ring *tx_ring = priv->tx_ring[TX][i];
+
 		if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, i)))
 			continue;
 		en_warn(priv, "TX timeout on queue: %d, QP: 0x%x, CQ: 0x%x, Cons: 0x%x, Prod: 0x%x\n",
-			i, priv->tx_ring[i]->qpn, priv->tx_ring[i]->cqn,
-			priv->tx_ring[i]->cons, priv->tx_ring[i]->prod);
+			i, tx_ring->qpn, tx_ring->cqn,
+			tx_ring->cons, tx_ring->prod);
 	}
 
 	priv->port_stats.tx_timeout++;
@@ -1328,7 +1330,7 @@ static void mlx4_en_tx_timeout(struct net_device *dev)
 static void mlx4_en_set_default_moderation(struct mlx4_en_priv *priv)
 {
 	struct mlx4_en_cq *cq;
-	int i;
+	int i, t;
 
 	/* If we haven't received a specific coalescing setting
 	 * (module param), we set the moderation parameters as follows:
@@ -1353,10 +1355,12 @@ static void mlx4_en_set_default_moderation(struct mlx4_en_priv *priv)
 		priv->last_moder_bytes[i] = 0;
 	}
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		cq = priv->tx_cq[i];
-		cq->moder_cnt = priv->tx_frames;
-		cq->moder_time = priv->tx_usecs;
+	for (t = 0 ; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			cq = priv->tx_cq[t][i];
+			cq->moder_cnt = priv->tx_frames;
+			cq->moder_time = priv->tx_usecs;
+		}
 	}
 
 	/* Reset auto-moderation params */
@@ -1526,19 +1530,13 @@ static void mlx4_en_free_affinity_hint(struct mlx4_en_priv *priv, int ring_idx)
 static void mlx4_en_init_recycle_ring(struct mlx4_en_priv *priv,
 				      int tx_ring_idx)
 {
-	struct mlx4_en_tx_ring *tx_ring = priv->tx_ring[tx_ring_idx];
-	int rr_index;
+	struct mlx4_en_tx_ring *tx_ring = priv->tx_ring[TX_XDP][tx_ring_idx];
+	int rr_index = tx_ring_idx;
 
-	rr_index = (priv->xdp_ring_num - priv->tx_ring_num) + tx_ring_idx;
-	if (rr_index >= 0) {
-		tx_ring->free_tx_desc = mlx4_en_recycle_tx_desc;
-		tx_ring->recycle_ring = priv->rx_ring[rr_index];
-		en_dbg(DRV, priv,
-		       "Set tx_ring[%d]->recycle_ring = rx_ring[%d]\n",
-		       tx_ring_idx, rr_index);
-	} else {
-		tx_ring->recycle_ring = NULL;
-	}
+	tx_ring->free_tx_desc = mlx4_en_recycle_tx_desc;
+	tx_ring->recycle_ring = priv->rx_ring[rr_index];
+	en_dbg(DRV, priv, "Set tx_ring[%d][%d]->recycle_ring = rx_ring[%d]\n",
+	       TX_XDP, tx_ring_idx, rr_index);
 }
 
 int mlx4_en_start_port(struct net_device *dev)
@@ -1548,9 +1546,8 @@ int mlx4_en_start_port(struct net_device *dev)
 	struct mlx4_en_cq *cq;
 	struct mlx4_en_tx_ring *tx_ring;
 	int rx_index = 0;
-	int tx_index = 0;
 	int err = 0;
-	int i;
+	int i, t;
 	int j;
 	u8 mc_list[16] = {0};
 
@@ -1635,43 +1632,51 @@ int mlx4_en_start_port(struct net_device *dev)
 		goto rss_err;
 
 	/* Configure tx cq's and rings */
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		/* Configure cq */
-		cq = priv->tx_cq[i];
-		err = mlx4_en_activate_cq(priv, cq, i);
-		if (err) {
-			en_err(priv, "Failed allocating Tx CQ\n");
-			goto tx_err;
-		}
-		err = mlx4_en_set_cq_moder(priv, cq);
-		if (err) {
-			en_err(priv, "Failed setting cq moderation parameters\n");
-			mlx4_en_deactivate_cq(priv, cq);
-			goto tx_err;
-		}
-		en_dbg(DRV, priv, "Resetting index of collapsed CQ:%d to -1\n", i);
-		cq->buf->wqe_index = cpu_to_be16(0xffff);
+	for (t = 0 ; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		u8 num_tx_rings_p_up = t == TX ? priv->num_tx_rings_p_up : 1;
 
-		/* Configure ring */
-		tx_ring = priv->tx_ring[i];
-		err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn,
-			i / priv->num_tx_rings_p_up);
-		if (err) {
-			en_err(priv, "Failed allocating Tx ring\n");
-			mlx4_en_deactivate_cq(priv, cq);
-			goto tx_err;
-		}
-		tx_ring->tx_queue = netdev_get_tx_queue(dev, i);
-
-		mlx4_en_init_recycle_ring(priv, i);
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			/* Configure cq */
+			cq = priv->tx_cq[t][i];
+			err = mlx4_en_activate_cq(priv, cq, i);
+			if (err) {
+				en_err(priv, "Failed allocating Tx CQ\n");
+				goto tx_err;
+			}
+			err = mlx4_en_set_cq_moder(priv, cq);
+			if (err) {
+				en_err(priv, "Failed setting cq moderation parameters\n");
+				mlx4_en_deactivate_cq(priv, cq);
+				goto tx_err;
+			}
+			en_dbg(DRV, priv,
+			       "Resetting index of collapsed CQ:%d to -1\n", i);
+			cq->buf->wqe_index = cpu_to_be16(0xffff);
+
+			/* Configure ring */
+			tx_ring = priv->tx_ring[t][i];
+			err = mlx4_en_activate_tx_ring(priv, tx_ring,
+						       cq->mcq.cqn,
+						       i / num_tx_rings_p_up);
+			if (err) {
+				en_err(priv, "Failed allocating Tx ring\n");
+				mlx4_en_deactivate_cq(priv, cq);
+				goto tx_err;
+			}
+			if (t != TX_XDP) {
+				tx_ring->tx_queue = netdev_get_tx_queue(dev, i);
+				tx_ring->recycle_ring = NULL;
+			} else {
+				mlx4_en_init_recycle_ring(priv, i);
+			}
 
-		/* Arm CQ for TX completions */
-		mlx4_en_arm_cq(priv, cq);
+			/* Arm CQ for TX completions */
+			mlx4_en_arm_cq(priv, cq);
 
-		/* Set initial ownership of all Tx TXBBs to SW (1) */
-		for (j = 0; j < tx_ring->buf_size; j += STAMP_STRIDE)
-			*((u32 *) (tx_ring->buf + j)) = 0xffffffff;
-		++tx_index;
+			/* Set initial ownership of all Tx TXBBs to SW (1) */
+			for (j = 0; j < tx_ring->buf_size; j += STAMP_STRIDE)
+				*((u32 *)(tx_ring->buf + j)) = 0xffffffff;
+		}
 	}
 
 	/* Configure port */
@@ -1746,9 +1751,18 @@ int mlx4_en_start_port(struct net_device *dev)
 	return 0;
 
 tx_err:
-	while (tx_index--) {
-		mlx4_en_deactivate_tx_ring(priv, priv->tx_ring[tx_index]);
-		mlx4_en_deactivate_cq(priv, priv->tx_cq[tx_index]);
+	if (t == MLX4_EN_NUM_TX_TYPES) {
+		t--;
+		i = priv->tx_ring_num[t];
+	}
+	while (t >= 0) {
+		while (i--) {
+			mlx4_en_deactivate_tx_ring(priv, priv->tx_ring[t][i]);
+			mlx4_en_deactivate_cq(priv, priv->tx_cq[t][i]);
+		}
+		if (!t--)
+			break;
+		i = priv->tx_ring_num[t];
 	}
 	mlx4_en_destroy_drop_qp(priv);
 rss_err:
@@ -1773,7 +1787,7 @@ void mlx4_en_stop_port(struct net_device *dev, int detach)
 	struct mlx4_en_dev *mdev = priv->mdev;
 	struct mlx4_en_mc_list *mclist, *tmp;
 	struct ethtool_flow_id *flow, *tmp_flow;
-	int i;
+	int i, t;
 	u8 mc_list[16] = {0};
 
 	if (!priv->port_up) {
@@ -1859,14 +1873,17 @@ void mlx4_en_stop_port(struct net_device *dev, int detach)
 	mlx4_en_destroy_drop_qp(priv);
 
 	/* Free TX Rings */
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		mlx4_en_deactivate_tx_ring(priv, priv->tx_ring[i]);
-		mlx4_en_deactivate_cq(priv, priv->tx_cq[i]);
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			mlx4_en_deactivate_tx_ring(priv, priv->tx_ring[t][i]);
+			mlx4_en_deactivate_cq(priv, priv->tx_cq[t][i]);
+		}
 	}
 	msleep(10);
 
-	for (i = 0; i < priv->tx_ring_num; i++)
-		mlx4_en_free_tx_buf(dev, priv->tx_ring[i]);
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++)
+		for (i = 0; i < priv->tx_ring_num[t]; i++)
+			mlx4_en_free_tx_buf(dev, priv->tx_ring[t][i]);
 
 	if (mdev->dev->caps.steering_mode != MLX4_STEERING_MODE_A0)
 		mlx4_en_delete_rss_steer_rules(priv);
@@ -1915,6 +1932,7 @@ static void mlx4_en_clear_stats(struct net_device *dev)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_en_dev *mdev = priv->mdev;
+	struct mlx4_en_tx_ring **tx_ring;
 	int i;
 
 	if (!mlx4_is_slave(mdev->dev))
@@ -1932,15 +1950,16 @@ static void mlx4_en_clear_stats(struct net_device *dev)
 	       sizeof(priv->tx_priority_flowstats));
 	memset(&priv->pf_stats, 0, sizeof(priv->pf_stats));
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		priv->tx_ring[i]->bytes = 0;
-		priv->tx_ring[i]->packets = 0;
-		priv->tx_ring[i]->tx_csum = 0;
-		priv->tx_ring[i]->tx_dropped = 0;
-		priv->tx_ring[i]->queue_stopped = 0;
-		priv->tx_ring[i]->wake_queue = 0;
-		priv->tx_ring[i]->tso_packets = 0;
-		priv->tx_ring[i]->xmit_more = 0;
+	tx_ring = priv->tx_ring[TX];
+	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
+		tx_ring[i]->bytes = 0;
+		tx_ring[i]->packets = 0;
+		tx_ring[i]->tx_csum = 0;
+		tx_ring[i]->tx_dropped = 0;
+		tx_ring[i]->queue_stopped = 0;
+		tx_ring[i]->wake_queue = 0;
+		tx_ring[i]->tso_packets = 0;
+		tx_ring[i]->xmit_more = 0;
 	}
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		priv->rx_ring[i]->bytes = 0;
@@ -1996,17 +2015,20 @@ static int mlx4_en_close(struct net_device *dev)
 
 static void mlx4_en_free_resources(struct mlx4_en_priv *priv)
 {
-	int i;
+	int i, t;
 
 #ifdef CONFIG_RFS_ACCEL
 	priv->dev->rx_cpu_rmap = NULL;
 #endif
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		if (priv->tx_ring && priv->tx_ring[i])
-			mlx4_en_destroy_tx_ring(priv, &priv->tx_ring[i]);
-		if (priv->tx_cq && priv->tx_cq[i])
-			mlx4_en_destroy_cq(priv, &priv->tx_cq[i]);
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			if (priv->tx_ring[t] && priv->tx_ring[t][i])
+				mlx4_en_destroy_tx_ring(priv,
+							&priv->tx_ring[t][i]);
+			if (priv->tx_cq[t] && priv->tx_cq[t][i])
+				mlx4_en_destroy_cq(priv, &priv->tx_cq[t][i]);
+		}
 	}
 
 	for (i = 0; i < priv->rx_ring_num; i++) {
@@ -2022,20 +2044,22 @@ static void mlx4_en_free_resources(struct mlx4_en_priv *priv)
 static int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
 {
 	struct mlx4_en_port_profile *prof = priv->prof;
-	int i;
+	int i, t;
 	int node;
 
 	/* Create tx Rings */
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		node = cpu_to_node(i % num_online_cpus());
-		if (mlx4_en_create_cq(priv, &priv->tx_cq[i],
-				      prof->tx_ring_size, i, TX, node))
-			goto err;
-
-		if (mlx4_en_create_tx_ring(priv, &priv->tx_ring[i],
-					   prof->tx_ring_size, TXBB_SIZE,
-					   node, i))
-			goto err;
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			node = cpu_to_node(i % num_online_cpus());
+			if (mlx4_en_create_cq(priv, &priv->tx_cq[t][i],
+					      prof->tx_ring_size, i, t, node))
+				goto err;
+
+			if (mlx4_en_create_tx_ring(priv, &priv->tx_ring[t][i],
+						   prof->tx_ring_size,
+						   TXBB_SIZE, node, i))
+				goto err;
+		}
 	}
 
 	/* Create rx Rings */
@@ -2067,11 +2091,14 @@ static int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
 		if (priv->rx_cq[i])
 			mlx4_en_destroy_cq(priv, &priv->rx_cq[i]);
 	}
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		if (priv->tx_ring[i])
-			mlx4_en_destroy_tx_ring(priv, &priv->tx_ring[i]);
-		if (priv->tx_cq[i])
-			mlx4_en_destroy_cq(priv, &priv->tx_cq[i]);
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		for (i = 0; i < priv->tx_ring_num[t]; i++) {
+			if (priv->tx_ring[t][i])
+				mlx4_en_destroy_tx_ring(priv,
+							&priv->tx_ring[t][i]);
+			if (priv->tx_cq[t][i])
+				mlx4_en_destroy_cq(priv, &priv->tx_cq[t][i]);
+		}
 	}
 	return -ENOMEM;
 }
@@ -2088,10 +2115,11 @@ static int mlx4_en_copy_priv(struct mlx4_en_priv *dst,
 			     struct mlx4_en_priv *src,
 			     struct mlx4_en_port_profile *prof)
 {
+	int t;
+
 	memcpy(&dst->hwtstamp_config, &prof->hwtstamp_config,
 	       sizeof(dst->hwtstamp_config));
 	dst->num_tx_rings_p_up = src->mdev->profile.num_tx_rings_p_up;
-	dst->tx_ring_num = prof->tx_ring_num;
 	dst->rx_ring_num = prof->rx_ring_num;
 	dst->flags = prof->flags;
 	dst->mdev = src->mdev;
@@ -2101,33 +2129,50 @@ static int mlx4_en_copy_priv(struct mlx4_en_priv *dst,
 	dst->stride = roundup_pow_of_two(sizeof(struct mlx4_en_rx_desc) +
 					 DS_SIZE * MLX4_EN_MAX_RX_FRAGS);
 
-	dst->tx_ring = kzalloc(sizeof(struct mlx4_en_tx_ring *) * MAX_TX_RINGS,
-				GFP_KERNEL);
-	if (!dst->tx_ring)
-		return -ENOMEM;
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		dst->tx_ring_num[t] = prof->tx_ring_num[t];
+		if (!dst->tx_ring_num[t])
+			continue;
 
-	dst->tx_cq = kzalloc(sizeof(struct mlx4_en_cq *) * MAX_TX_RINGS,
-			      GFP_KERNEL);
-	if (!dst->tx_cq) {
-		kfree(dst->tx_ring);
-		return -ENOMEM;
+		dst->tx_ring[t] = kzalloc(sizeof(struct mlx4_en_tx_ring *) *
+					  MAX_TX_RINGS, GFP_KERNEL);
+		if (!dst->tx_ring[t])
+			goto err_free_tx;
+
+		dst->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
+					MAX_TX_RINGS, GFP_KERNEL);
+		if (!dst->tx_cq[t]) {
+			kfree(dst->tx_ring[t]);
+			goto err_free_tx;
+		}
 	}
+
 	return 0;
+
+err_free_tx:
+	while (t--) {
+		kfree(dst->tx_ring[t]);
+		kfree(dst->tx_cq[t]);
+	}
+	return -ENOMEM;
 }
 
 static void mlx4_en_update_priv(struct mlx4_en_priv *dst,
 				struct mlx4_en_priv *src)
 {
+	int t;
 	memcpy(dst->rx_ring, src->rx_ring,
 	       sizeof(struct mlx4_en_rx_ring *) * src->rx_ring_num);
 	memcpy(dst->rx_cq, src->rx_cq,
 	       sizeof(struct mlx4_en_cq *) * src->rx_ring_num);
 	memcpy(&dst->hwtstamp_config, &src->hwtstamp_config,
 	       sizeof(dst->hwtstamp_config));
-	dst->tx_ring_num = src->tx_ring_num;
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		dst->tx_ring_num[t] = src->tx_ring_num[t];
+		dst->tx_ring[t] = src->tx_ring[t];
+		dst->tx_cq[t] = src->tx_cq[t];
+	}
 	dst->rx_ring_num = src->rx_ring_num;
-	dst->tx_ring = src->tx_ring;
-	dst->tx_cq = src->tx_cq;
 	memcpy(dst->prof, src->prof, sizeof(struct mlx4_en_port_profile));
 }
 
@@ -2135,14 +2180,18 @@ int mlx4_en_try_alloc_resources(struct mlx4_en_priv *priv,
 				struct mlx4_en_priv *tmp,
 				struct mlx4_en_port_profile *prof)
 {
+	int t;
+
 	mlx4_en_copy_priv(tmp, priv, prof);
 
 	if (mlx4_en_alloc_resources(tmp)) {
 		en_warn(priv,
 			"%s: Resource allocation failed, using previous configuration\n",
 			__func__);
-		kfree(tmp->tx_ring);
-		kfree(tmp->tx_cq);
+		for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+			kfree(tmp->tx_ring[t]);
+			kfree(tmp->tx_cq[t]);
+		}
 		return -ENOMEM;
 	}
 	return 0;
@@ -2161,6 +2210,7 @@ void mlx4_en_destroy_netdev(struct net_device *dev)
 	struct mlx4_en_dev *mdev = priv->mdev;
 	bool shutdown = mdev->dev->persist->interface_state &
 					    MLX4_INTERFACE_STATE_SHUTDOWN;
+	int t;
 
 	en_dbg(DRV, priv, "Destroying netdev on port:%d\n", priv->port);
 
@@ -2197,8 +2247,10 @@ void mlx4_en_destroy_netdev(struct net_device *dev)
 
 	mlx4_en_free_resources(priv);
 
-	kfree(priv->tx_ring);
-	kfree(priv->tx_cq);
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		kfree(priv->tx_ring[t]);
+		kfree(priv->tx_cq[t]);
+	}
 
 	if (!shutdown)
 		free_netdev(dev);
@@ -2214,7 +2266,7 @@ static int mlx4_en_change_mtu(struct net_device *dev, int new_mtu)
 	en_dbg(DRV, priv, "Change MTU called - current:%d new:%d\n",
 		 dev->mtu, new_mtu);
 
-	if (priv->xdp_ring_num && MLX4_EN_EFF_MTU(new_mtu) > FRAG_SZ0) {
+	if (priv->tx_ring_num[TX_XDP] && MLX4_EN_EFF_MTU(new_mtu) > FRAG_SZ0) {
 		en_err(priv, "MTU size:%d requires frags but XDP running\n",
 		       new_mtu);
 		return -EOPNOTSUPP;
@@ -2605,7 +2657,7 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
 static int mlx4_en_set_tx_maxrate(struct net_device *dev, int queue_index, u32 maxrate)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
-	struct mlx4_en_tx_ring *tx_ring = priv->tx_ring[queue_index];
+	struct mlx4_en_tx_ring *tx_ring = priv->tx_ring[TX][queue_index];
 	struct mlx4_update_qp_params params;
 	int err;
 
@@ -2633,18 +2685,21 @@ static int mlx4_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_en_dev *mdev = priv->mdev;
+	struct mlx4_en_port_profile new_prof;
 	struct bpf_prog *old_prog;
+	struct mlx4_en_priv *tmp;
+	int tx_changed = 0;
 	int xdp_ring_num;
 	int port_up = 0;
 	int err;
 	int i;
 
-	xdp_ring_num = prog ? ALIGN(priv->rx_ring_num, MLX4_EN_NUM_UP) : 0;
+	xdp_ring_num = prog ? priv->rx_ring_num : 0;
 
 	/* No need to reconfigure buffers when simply swapping the
 	 * program for a new one.
 	 */
-	if (priv->xdp_ring_num == xdp_ring_num) {
+	if (priv->tx_ring_num[TX_XDP] == xdp_ring_num) {
 		if (prog) {
 			prog = bpf_prog_add(prog, priv->rx_ring_num - 1);
 			if (IS_ERR(prog))
@@ -2668,28 +2723,41 @@ static int mlx4_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 		return -EOPNOTSUPP;
 	}
 
-	if (priv->tx_ring_num < xdp_ring_num + MLX4_EN_NUM_UP) {
-		en_err(priv,
-		       "Minimum %d tx channels required to run XDP\n",
-		       (xdp_ring_num + MLX4_EN_NUM_UP) / MLX4_EN_NUM_UP);
-		return -EINVAL;
-	}
+	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
 
 	if (prog) {
 		prog = bpf_prog_add(prog, priv->rx_ring_num - 1);
-		if (IS_ERR(prog))
-			return PTR_ERR(prog);
+		if (IS_ERR(prog)) {
+			err = PTR_ERR(prog);
+			goto out;
+		}
 	}
 
 	mutex_lock(&mdev->state_lock);
+	memcpy(&new_prof, priv->prof, sizeof(struct mlx4_en_port_profile));
+	new_prof.tx_ring_num[TX_XDP] = xdp_ring_num;
+
+	if (priv->tx_ring_num[TX] + xdp_ring_num > MAX_TX_RINGS) {
+		tx_changed = 1;
+		new_prof.tx_ring_num[TX] =
+			MAX_TX_RINGS - ALIGN(xdp_ring_num, MLX4_EN_NUM_UP);
+		en_warn(priv, "Reducing the number of TX rings, to not exceed the max total rings number.\n");
+	}
+
+	err = mlx4_en_try_alloc_resources(priv, tmp, &new_prof);
+	if (err)
+		goto unlock_out;
+
 	if (priv->port_up) {
 		port_up = 1;
 		mlx4_en_stop_port(dev, 1);
 	}
 
-	priv->xdp_ring_num = xdp_ring_num;
-	netif_set_real_num_tx_queues(dev, priv->tx_ring_num -
-							priv->xdp_ring_num);
+	mlx4_en_safe_replace_resources(priv, tmp);
+	if (tx_changed)
+		netif_set_real_num_tx_queues(dev, priv->tx_ring_num[TX]);
 
 	for (i = 0; i < priv->rx_ring_num; i++) {
 		old_prog = rcu_dereference_protected(
@@ -2709,15 +2777,18 @@ static int mlx4_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 		}
 	}
 
+unlock_out:
 	mutex_unlock(&mdev->state_lock);
-	return 0;
+out:
+	kfree(tmp);
+	return err;
 }
 
 static bool mlx4_xdp_attached(struct net_device *dev)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 
-	return !!priv->xdp_ring_num;
+	return !!priv->tx_ring_num[TX_XDP];
 }
 
 static int mlx4_xdp(struct net_device *dev, struct netdev_xdp *xdp)
@@ -3061,7 +3132,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 {
 	struct net_device *dev;
 	struct mlx4_en_priv *priv;
-	int i;
+	int i, t;
 	int err;
 
 	dev = alloc_etherdev_mqs(sizeof(struct mlx4_en_priv),
@@ -3069,7 +3140,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	if (dev == NULL)
 		return -ENOMEM;
 
-	netif_set_real_num_tx_queues(dev, prof->tx_ring_num);
+	netif_set_real_num_tx_queues(dev, prof->tx_ring_num[TX]);
 	netif_set_real_num_rx_queues(dev, prof->rx_ring_num);
 
 	SET_NETDEV_DEV(dev, &mdev->dev->persist->pdev->dev);
@@ -3106,21 +3177,27 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	priv->ctrl_flags = cpu_to_be32(MLX4_WQE_CTRL_CQ_UPDATE |
 			MLX4_WQE_CTRL_SOLICITED);
 	priv->num_tx_rings_p_up = mdev->profile.num_tx_rings_p_up;
-	priv->tx_ring_num = prof->tx_ring_num;
 	priv->tx_work_limit = MLX4_EN_DEFAULT_TX_WORK;
 	netdev_rss_key_fill(priv->rss_key, sizeof(priv->rss_key));
 
-	priv->tx_ring = kzalloc(sizeof(struct mlx4_en_tx_ring *) * MAX_TX_RINGS,
-				GFP_KERNEL);
-	if (!priv->tx_ring) {
-		err = -ENOMEM;
-		goto out;
-	}
-	priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq *) * MAX_TX_RINGS,
-			      GFP_KERNEL);
-	if (!priv->tx_cq) {
-		err = -ENOMEM;
-		goto out;
+	for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
+		priv->tx_ring_num[t] = prof->tx_ring_num[t];
+		if (!priv->tx_ring_num[t])
+			continue;
+
+		priv->tx_ring[t] = kzalloc(sizeof(struct mlx4_en_tx_ring *) *
+					   MAX_TX_RINGS, GFP_KERNEL);
+		if (!priv->tx_ring[t]) {
+			err = -ENOMEM;
+			goto err_free_tx;
+		}
+		priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
+					 MAX_TX_RINGS, GFP_KERNEL);
+		if (!priv->tx_cq[t]) {
+			kfree(priv->tx_ring[t]);
+			err = -ENOMEM;
+			goto out;
+		}
 	}
 	priv->rx_ring_num = prof->rx_ring_num;
 	priv->cqe_factor = (mdev->dev->caps.cqe_size == 64) ? 1 : 0;
@@ -3203,7 +3280,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	else
 		dev->netdev_ops = &mlx4_netdev_ops;
 	dev->watchdog_timeo = MLX4_EN_WATCHDOG_TIMEOUT;
-	netif_set_real_num_tx_queues(dev, priv->tx_ring_num);
+	netif_set_real_num_tx_queues(dev, priv->tx_ring_num[TX]);
 	netif_set_real_num_rx_queues(dev, priv->rx_ring_num);
 
 	dev->ethtool_ops = &mlx4_en_ethtool_ops;
@@ -3303,7 +3380,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	netif_carrier_off(dev);
 	mlx4_en_set_default_moderation(priv);
 
-	en_warn(priv, "Using %d TX rings\n", prof->tx_ring_num);
+	en_warn(priv, "Using %d TX rings\n", prof->tx_ring_num[TX]);
 	en_warn(priv, "Using %d RX rings\n", prof->rx_ring_num);
 
 	mlx4_en_update_loopback_state(priv->dev, priv->dev->features);
@@ -3363,6 +3440,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 
 	return 0;
 
+err_free_tx:
+	while (t--) {
+		kfree(priv->tx_ring[t]);
+		kfree(priv->tx_cq[t]);
+	}
 out:
 	mlx4_en_destroy_netdev(dev);
 	return err;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.c b/drivers/net/ethernet/mellanox/mlx4/en_port.c
index 59473a0ebcdf..2e1ab4642569 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_port.c
@@ -196,8 +196,8 @@ int mlx4_en_DUMP_ETH_STATS(struct mlx4_en_dev *mdev, u8 port, u8 reset)
 	priv->port_stats.tso_packets = 0;
 	priv->port_stats.xmit_more = 0;
 
-	for (i = 0; i < priv->tx_ring_num; i++) {
-		const struct mlx4_en_tx_ring *ring = priv->tx_ring[i];
+	for (i = 0; i < priv->tx_ring_num[TX]; i++) {
+		const struct mlx4_en_tx_ring *ring = priv->tx_ring[TX][i];
 
 		stats->tx_packets += ring->packets;
 		stats->tx_bytes += ring->bytes;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index f2e8beddcf44..71196f68b55d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -788,7 +788,6 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 	struct bpf_prog *xdp_prog;
 	int doorbell_pending;
 	struct sk_buff *skb;
-	int tx_index;
 	int index;
 	int nr;
 	unsigned int length;
@@ -808,7 +807,6 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 	rcu_read_lock();
 	xdp_prog = rcu_dereference(ring->xdp_prog);
 	doorbell_pending = 0;
-	tx_index = (priv->tx_ring_num - priv->xdp_ring_num) + cq->ring;
 
 	/* We assume a 1:1 mapping between CQEs and Rx descriptors, so Rx
 	 * descriptor offset can be deduced from the CQE index instead of
@@ -905,7 +903,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 				break;
 			case XDP_TX:
 				if (likely(!mlx4_en_xmit_frame(frags, dev,
-							length, tx_index,
+							length, cq->ring,
 							&doorbell_pending)))
 					goto consumed;
 				goto xdp_drop; /* Drop on xmit failure */
@@ -1082,7 +1080,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 out:
 	rcu_read_unlock();
 	if (doorbell_pending)
-		mlx4_en_xmit_doorbell(priv->tx_ring[tx_index]);
+		mlx4_en_xmit_doorbell(priv->tx_ring[TX_XDP][cq->ring]);
 
 	AVG_PERF_COUNTER(priv->pstats.rx_coal_avg, polled);
 	mlx4_cq_set_ci(&cq->mcq);
@@ -1162,7 +1160,7 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
 	/* bpf requires buffers to be set up as 1 packet per page.
 	 * This only works when num_frags == 1.
 	 */
-	if (priv->xdp_ring_num) {
+	if (priv->tx_ring_num[TX_XDP]) {
 		dma_dir = PCI_DMA_BIDIRECTIONAL;
 		/* This will gain efficient xdp frame recycling at the expense
 		 * of more costly truesize accounting
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index e2509bba3e7c..95dc864bb2f7 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -392,7 +392,8 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 		cnt++;
 	}
 
-	netdev_tx_reset_queue(ring->tx_queue);
+	if (ring->tx_queue)
+		netdev_tx_reset_queue(ring->tx_queue);
 
 	if (cnt)
 		en_dbg(DRV, priv, "Freed %d uncompleted tx descriptors\n", cnt);
@@ -405,7 +406,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_cq *mcq = &cq->mcq;
-	struct mlx4_en_tx_ring *ring = priv->tx_ring[cq->ring];
+	struct mlx4_en_tx_ring *ring = priv->tx_ring[cq->type][cq->ring];
 	struct mlx4_cqe *cqe;
 	u16 index;
 	u16 new_index, ring_index, stamp_index;
@@ -807,7 +808,7 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 	bool bf_ok;
 
 	tx_ind = skb_get_queue_mapping(skb);
-	ring = priv->tx_ring[tx_ind];
+	ring = priv->tx_ring[TX][tx_ind];
 
 	if (!priv->port_up)
 		goto tx_drop;
@@ -1101,7 +1102,7 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_alloc *frame,
 	BUILD_BUG_ON_MSG(ALIGN(CTRL_SIZE + DS_SIZE, TXBB_SIZE) != TXBB_SIZE,
 			 "mlx4_en_xmit_frame requires minimum size tx desc");
 
-	ring = priv->tx_ring[tx_ind];
+	ring = priv->tx_ring[TX_XDP][tx_ind];
 
 	if (!priv->port_up)
 		goto tx_drop;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 83c914a79f14..6e0693659f85 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -207,8 +207,10 @@ enum {
  */
 
 enum cq_type {
+	/* keep tx types first */
 	TX,
 	TX_XDP,
+#define MLX4_EN_NUM_TX_TYPES (TX_XDP + 1)
 	RX,
 };
 
@@ -373,7 +375,7 @@ struct mlx4_en_cq {
 
 struct mlx4_en_port_profile {
 	u32 flags;
-	u32 tx_ring_num;
+	u32 tx_ring_num[MLX4_EN_NUM_TX_TYPES];
 	u32 rx_ring_num;
 	u32 tx_ring_size;
 	u32 rx_ring_size;
@@ -570,17 +572,16 @@ struct mlx4_en_priv {
 	u32 flags;
 	u8 num_tx_rings_p_up;
 	u32 tx_work_limit;
-	u32 tx_ring_num;
+	u32 tx_ring_num[MLX4_EN_NUM_TX_TYPES];
 	u32 rx_ring_num;
 	u32 rx_skb_size;
 	struct mlx4_en_frag_info frag_info[MLX4_EN_MAX_RX_FRAGS];
 	u16 num_frags;
 	u16 log_rx_info;
-	int xdp_ring_num;
 
-	struct mlx4_en_tx_ring **tx_ring;
+	struct mlx4_en_tx_ring **tx_ring[MLX4_EN_NUM_TX_TYPES];
 	struct mlx4_en_rx_ring *rx_ring[MAX_RX_RINGS];
-	struct mlx4_en_cq **tx_cq;
+	struct mlx4_en_cq **tx_cq[MLX4_EN_NUM_TX_TYPES];
 	struct mlx4_en_cq *rx_cq[MAX_RX_RINGS];
 	struct mlx4_qp drop_qp;
 	struct work_struct rx_mode_task;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next V3 1/3] net/mlx4_en: Add TX_XDP for CQ types
From: Tariq Toukan @ 2016-11-02 15:12 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Brenden Blanco, Alexei Starovoitov,
	Saeed Mahameed, Tariq Toukan
In-Reply-To: <1478099545-24181-1-git-send-email-tariqt@mellanox.com>

Support XDP CQ type, and refactor the CQ type enum.
Rename the is_tx field to match the change.

Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_cq.c   | 18 +++++++++---------
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |  7 ++++---
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index e3be7e44ff51..1427311a9640 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -65,7 +65,7 @@ int mlx4_en_create_cq(struct mlx4_en_priv *priv,
 	cq->buf_size = cq->size * mdev->dev->caps.cqe_size;
 
 	cq->ring = ring;
-	cq->is_tx = mode;
+	cq->type = mode;
 	cq->vector = mdev->dev->caps.num_comp_vectors;
 
 	/* Allocate HW buffers on provided NUMA node.
@@ -104,7 +104,7 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 	*cq->mcq.arm_db    = 0;
 	memset(cq->buf, 0, cq->buf_size);
 
-	if (cq->is_tx == RX) {
+	if (cq->type == RX) {
 		if (!mlx4_is_eq_vector_valid(mdev->dev, priv->port,
 					     cq->vector)) {
 			cq->vector = cpumask_first(priv->rx_ring[cq->ring]->affinity_mask);
@@ -141,11 +141,11 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 		cq->vector = rx_cq->vector;
 	}
 
-	if (!cq->is_tx)
+	if (cq->type == RX)
 		cq->size = priv->rx_ring[cq->ring]->actual_size;
 
-	if ((cq->is_tx && priv->hwtstamp_config.tx_type) ||
-	    (!cq->is_tx && priv->hwtstamp_config.rx_filter))
+	if ((cq->type != RX && priv->hwtstamp_config.tx_type) ||
+	    (cq->type == RX && priv->hwtstamp_config.rx_filter))
 		timestamp_en = 1;
 
 	err = mlx4_cq_alloc(mdev->dev, cq->size, &cq->wqres.mtt,
@@ -154,10 +154,10 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
 	if (err)
 		goto free_eq;
 
-	cq->mcq.comp  = cq->is_tx ? mlx4_en_tx_irq : mlx4_en_rx_irq;
+	cq->mcq.comp  = cq->type != RX ? mlx4_en_tx_irq : mlx4_en_rx_irq;
 	cq->mcq.event = mlx4_en_cq_event;
 
-	if (cq->is_tx)
+	if (cq->type != RX)
 		netif_tx_napi_add(cq->dev, &cq->napi, mlx4_en_poll_tx_cq,
 				  NAPI_POLL_WEIGHT);
 	else
@@ -181,7 +181,7 @@ void mlx4_en_destroy_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq **pcq)
 
 	mlx4_free_hwq_res(mdev->dev, &cq->wqres, cq->buf_size);
 	if (mlx4_is_eq_vector_valid(mdev->dev, priv->port, cq->vector) &&
-	    cq->is_tx == RX)
+	    cq->type == RX)
 		mlx4_release_eq(priv->mdev->dev, cq->vector);
 	cq->vector = 0;
 	cq->buf_size = 0;
@@ -193,7 +193,7 @@ void mlx4_en_destroy_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq **pcq)
 void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq)
 {
 	napi_disable(&cq->napi);
-	if (!cq->is_tx) {
+	if (cq->type == RX) {
 		napi_hash_del(&cq->napi);
 		synchronize_rcu();
 	}
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index a3528dd1e72e..83c914a79f14 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -207,8 +207,9 @@ enum {
  */
 
 enum cq_type {
-	RX = 0,
-	TX = 1,
+	TX,
+	TX_XDP,
+	RX,
 };
 
 
@@ -361,7 +362,7 @@ struct mlx4_en_cq {
 	int size;
 	int buf_size;
 	int vector;
-	enum cq_type is_tx;
+	enum cq_type type;
 	u16 moder_time;
 	u16 moder_cnt;
 	struct mlx4_cqe *buf;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [patch net-next 0/2] Fixes for raw diag sockets handling
From: David Ahern @ 2016-11-02 15:10 UTC (permalink / raw)
  To: Cyrill Gorcunov, netdev
  Cc: Eric Dumazet, David S. Miller, Andrey Vagin, Stephen Hemminger
In-Reply-To: <20161102123630.784612652@openvz.org>

On 11/2/16 6:36 AM, Cyrill Gorcunov wrote:
> Also I have a question about sockets lookup not for raw diag only
> (though I didn't modify lookup procedure) but in general: the structure
> inet_diag_req_v2 has inet_diag_sockid::idiag_if member which supposed to
> carry interface index from userspace request.
> 
> Then for example in INET_MATCH (include/net/inet_hashtables.h),
> the __dif parameter (which is @idiag_if) compared with @sk_bound_dev_if
> *iif* the sk_bound_dev_if has been ever set. Thus if say someone
> looks up for paticular device with specified index if the
> rest of parameters match and SO_BINDTODEVICE never been called
> for this device we return the socket even if idiag_if is not zero.
> Is it supposed to be so? Or I miss something obvious?
> 
> I mean this snippet
> 
> 
> 	 (!(__sk)->sk_bound_dev_if	||				\
> 	   ((__sk)->sk_bound_dev_if == (__dif))) 	&&		\
> 
> when someone calls for destory sockets on particular interface and
> @__dif != 0 the match may return socket where sk_bound_dev_if = 0
> instead of completely matching one. Isn't it?

yes. I recently added an exact_dif to the lookup for listener sockets (see compute_score). Something like that could be added to INET_MATCH.

^ permalink raw reply

* [PATCH iproute2] tc: flower: Fix usage message
From: Paul Blakey @ 2016-11-02 15:09 UTC (permalink / raw)
  To: netdev; +Cc: Or Gerlitz, Paul Blakey

Remove left over usage from removal of eth_type argument.

Fixes: 488b41d020fb ('tc: flower no need to specify the ethertype')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
 man/man8/tc-flower.8 | 9 ---------
 tc/f_flower.c        | 3 +--
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 74f7664..16ef261 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -23,8 +23,6 @@ flower \- flow based traffic control filter
 .R " | { "
 .BR dst_mac " | " src_mac " } "
 .IR mac_address " | "
-.BR eth_type " { " ipv4 " | " ipv6 " | " 802.1Q " | "
-.IR ETH_TYPE " } | "
 .B vlan_id
 .IR VID " | "
 .B vlan_prio
@@ -75,13 +73,6 @@ Do not process filter by hardware.
 .BI src_mac " mac_address"
 Match on source or destination MAC address.
 .TP
-.BI eth_type " ETH_TYPE"
-Match on the next protocol.
-.I ETH_TYPE
-may be either
-.BR ipv4 , ipv6 , 802.1Q ,
-or an unsigned 16bit value in hexadecimal format.
-.TP
 .BI vlan_id " VID"
 Match on vlan tag id.
 .I VID
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 2d31d1a..f39b1f7 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -36,7 +36,6 @@ static void explain(void)
 	fprintf(stderr, "                       vlan_ethtype [ ipv4 | ipv6 | ETH-TYPE ] |\n");
 	fprintf(stderr, "                       dst_mac MAC-ADDR |\n");
 	fprintf(stderr, "                       src_mac MAC-ADDR |\n");
-	fprintf(stderr, "                       [ipv4 | ipv6 ] |\n");
 	fprintf(stderr, "                       ip_proto [tcp | udp | IP-PROTO ] |\n");
 	fprintf(stderr, "                       dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
 	fprintf(stderr, "                       src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
@@ -45,7 +44,7 @@ static void explain(void)
 	fprintf(stderr,	"       FILTERID := X:Y:Z\n");
 	fprintf(stderr,	"       ACTION-SPEC := ... look at individual actions\n");
 	fprintf(stderr,	"\n");
-	fprintf(stderr,	"NOTE: CLASSID, ETH-TYPE, IP-PROTO are parsed as hexadecimal input.\n");
+	fprintf(stderr,	"NOTE: CLASSID, IP-PROTO are parsed as hexadecimal input.\n");
 	fprintf(stderr,	"NOTE: There can be only used one mask per one prio. If user needs\n");
 	fprintf(stderr,	"      to specify different mask, he has to use different prio.\n");
 }
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next 3/3] tools lib bpf: Sync bpf_map_def with tc
From: Daniel Borkmann @ 2016-11-02 15:08 UTC (permalink / raw)
  To: Joe Stringer; +Cc: netdev, wangnan0, ast
In-Reply-To: <5819F461.7050309@iogearbox.net>

On 11/02/2016 03:12 PM, Daniel Borkmann wrote:
> On 11/02/2016 05:09 AM, Joe Stringer wrote:
>> On 1 November 2016 at 20:09, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 10/31/2016 07:39 PM, Joe Stringer wrote:
>>>>
>>>> TC uses a slightly different map layout in its ELFs. Update libbpf to
>>>> use the same definition so that ELFs may be built using libbpf and
>>>> loaded using tc.
>>>>
>>>> Signed-off-by: Joe Stringer <joe@ovn.org>
>>>> ---
>>>>    tools/lib/bpf/libbpf.h | 11 +++++++----
>>>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>>>> index dd7a513efb10..ea70c2744f8c 100644
>>>> --- a/tools/lib/bpf/libbpf.h
>>>> +++ b/tools/lib/bpf/libbpf.h
>>>> @@ -181,10 +181,13 @@ bool bpf_program__is_kprobe(struct bpf_program
>>>> *prog);
>>>>     * and will be treated as an error due to -Werror.
>>>>     */
>>>>    struct bpf_map_def {
>>>> -       unsigned int type;
>>>> -       unsigned int key_size;
>>>> -       unsigned int value_size;
>>>> -       unsigned int max_entries;
>>>> +       uint32_t type;
>>>> +       uint32_t key_size;
>>>> +       uint32_t value_size;
>>>> +       uint32_t max_entries;
>>>> +       uint32_t flags;
>>>> +       uint32_t id;
>>>> +       uint32_t pinning;
>>>>    };
>>>>
>>>>    /*
>>>
>>> I think the problem is that this would break existing obj files that have
>>> been compiled with the current struct bpf_map_def (besides libbpf not having
>>> a use for the last two members right now).
>>
>> Right, this is a problem. I wasn't sure whether libbpf was yet at a
>> stage where it tries to retain compatibility with binaries compiled
>> against older kernels.
>>
>>> For tc, we have a refactoring of the tc_bpf.c bits that generalizes them so
>>> we can move these bits into iproute2 lib part and add new BPF types really
>>> easily. What I did along with that is to implement a map compat mode, where
>>> it detects the size of struct bpf_elf_map (or however you want to name it)
>>> from the obj file and fixes up the missing members with some reasonable
>>> default,
>>> so these programs can still be loaded. Thus, the sample code using the
>>> current
>>> struct bpf_map_def will then work with tc as well. (I'll post the iproute2
>>> patch early next week.)
>>
>> Are you encoding the number of maps into the start of the maps section
>> in the ELF then using that to divide out and determine the size?
>
> No. It's walking the symbol table and simply counts the number of symbols
> that are present in the maps section with correct st_info attribution. It
> works because that section name is fixed ABI for all cases and really per
> definition only map structs are present there. The minimum attributes which
> are allowed to be loaded are type, key_size, value_size and max_entries.
>
>> I look forward to your patches. Maybe if TC is more tolerant of other
>> map definition sizes then this patch is less relevant.

Just a thought (not really related to your set though): Perhaps it makes sense
for the bpflib, to also add an option where the user passes a callback to parse
the map section itself if something else than struct bpf_map_def is expected in
the obj, and then have a way to access the meta data along with the fd via lib
api later on. Perhaps that would also include another callback passed to the
lib that would take care to invoke bpf(2) for map creation, which could be useful
if some custom interaction with bpffs is desired.

^ 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