* Re: [PATCH v2] net: Fix typos and whitespace.
From: David Miller @ 2016-03-23 19:04 UTC (permalink / raw)
To: bhelgaas; +Cc: netdev, linux-kernel
In-Reply-To: <20160323184723.2198.94949.stgit@bhelgaas-glaptop2.roam.corp.google.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Wed, 23 Mar 2016 13:47:23 -0500
> Fix typos. Capitalize CPU, NAPI, RCU consistently. Align structure
> indentation. No functional change intended; only comment and whitespace
> changes.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Applied, thank you.
^ permalink raw reply
* Re: [RFC PATCH 8/9] i40e/i40evf: Add support for GSO partial with UDP_TUNNEL_CSUM and GRE_CSUM
From: Jesse Gross @ 2016-03-23 19:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: Edward Cree, Linux Kernel Network Developers, David Miller,
Alexander Duyck, Tom Herbert
In-Reply-To: <20160318232528.14955.87859.stgit@localhost.localdomain>
On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 39b0009253c2..ac3964a9f5c0 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -9074,7 +9075,12 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
> if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
> netdev->features |= NETIF_F_NTUPLE;
> if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
> - netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
> + netdev->features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
Looks like there might be a typo - I guess this is supposed to still
OR with the previous features instead of replacing them?
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: Sergei Shtylyov @ 2016-03-23 19:42 UTC (permalink / raw)
To: Sebastian Frias, Uwe Kleine-König
Cc: Daniel Mack, David S. Miller, netdev, lkml, mason,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl, Linus Walleij
In-Reply-To: <56F274A5.6030502@laposte.net>
Hello.
On 03/23/2016 01:49 PM, Sebastian Frias wrote:
> This removes the dependency on GPIOLIB for non faulty PHYs.
>
> Indeed, without this patch, if GPIOLIB is not selected
> devm_gpiod_get_optional() will return -ENOSYS and the driver probe
> call will fail, regardless of the actual PHY hardware.
>
> Out of the 3 PHYs supported by this driver (AT8030, AT8031, AT8035),
> only AT8030 presents the issues that commit 13a56b449325 ("net: phy:
> at803x: Add support for hardware reset") attempts to work-around by
> using a 'reset' GPIO line.
>
> Hence, only AT8030 should depend on GPIOLIB operating properly.
>
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
[...]
What I don't understand is why the link_change_notify() method ptr is
populated for all 3 supported chips while only being needed on 8030...
MBR, Sergei
^ permalink raw reply
* [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Petri Gynther @ 2016-03-23 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli, jaedon.shin, edumazet, Petri Gynther
1. Add bytes_compl local variable to collect transmitted bytes
- dev->stats updates can then be moved outside the while-loop
- bytes_compl is also needed for future BQL support
2. When bcmgenet device uses Tx checksum offload, each transmitted
skb has an extra 64-byte header prepended to it. This needs to
be deducted when counting transmitted bytes.
3. skb->len covers the entire length of skb, whether it is linear or
fragmented. Thus, when we clean the fragments, do not increase
transmitted bytes.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 6746fd0..9990582 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
struct enet_cb *tx_cb_ptr;
struct netdev_queue *txq;
unsigned int pkts_compl = 0;
+ unsigned int bytes_compl = 0;
unsigned int c_index;
unsigned int txbds_ready;
unsigned int txbds_processed = 0;
@@ -1193,16 +1194,14 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
if (tx_cb_ptr->skb) {
pkts_compl++;
- dev->stats.tx_packets++;
- dev->stats.tx_bytes += tx_cb_ptr->skb->len;
+ bytes_compl += tx_cb_ptr->skb->len -
+ (priv->desc_64b_en ? 64 : 0);
dma_unmap_single(&dev->dev,
dma_unmap_addr(tx_cb_ptr, dma_addr),
dma_unmap_len(tx_cb_ptr, dma_len),
DMA_TO_DEVICE);
bcmgenet_free_cb(tx_cb_ptr);
} else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) {
- dev->stats.tx_bytes +=
- dma_unmap_len(tx_cb_ptr, dma_len);
dma_unmap_page(&dev->dev,
dma_unmap_addr(tx_cb_ptr, dma_addr),
dma_unmap_len(tx_cb_ptr, dma_len),
@@ -1220,6 +1219,9 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
ring->free_bds += txbds_processed;
ring->c_index = (ring->c_index + txbds_processed) & DMA_C_INDEX_MASK;
+ dev->stats.tx_packets += pkts_compl;
+ dev->stats.tx_bytes += bytes_compl;
+
if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
txq = netdev_get_tx_queue(dev, ring->queue);
if (netif_tx_queue_stopped(txq))
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH net 2/2] net: bcmgenet: fix skb_len in bcmgenet_xmit_single()
From: Petri Gynther @ 2016-03-23 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli, jaedon.shin, edumazet, Petri Gynther
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
skb_len needs to be skb_headlen(skb) in bcmgenet_xmit_single().
Fragmented skbs can have only Ethernet + IP + TCP headers (14+20+20=54 bytes)
in the linear buffer, followed by the rest in fragments. Bumping skb_len to
ETH_ZLEN would be incorrect for this case, as it would introduce garbage
between TCP header and the fragment data.
This also works with regular/non-fragmented small packets < ETH_ZLEN bytes.
Successfully tested this on GENETv3 with 42-byte ARP frames.
For testing, I used:
ethtool -K eth0 tx-checksum-ipv4 off
ethtool -K eth0 tx-checksum-ipv6 off
echo 0 > /proc/sys/net/ipv4/tcp_timestamps
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 9990582..00da239 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1298,7 +1298,7 @@ static int bcmgenet_xmit_single(struct net_device *dev,
tx_cb_ptr->skb = skb;
- skb_len = skb_headlen(skb) < ETH_ZLEN ? ETH_ZLEN : skb_headlen(skb);
+ skb_len = skb_headlen(skb);
mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
ret = dma_mapping_error(kdev, mapping);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [RFC PATCH 8/9] i40e/i40evf: Add support for GSO partial with UDP_TUNNEL_CSUM and GRE_CSUM
From: Alexander Duyck @ 2016-03-23 20:21 UTC (permalink / raw)
To: Jesse Gross
Cc: Alexander Duyck, Edward Cree, Linux Kernel Network Developers,
David Miller, Tom Herbert
In-Reply-To: <CAEh+42jxjkhoTeC0GK91pmX6XUpnMAxnA-pRhbG0kkQFfa2WXQ@mail.gmail.com>
On Wed, Mar 23, 2016 at 12:35 PM, Jesse Gross <jesse@kernel.org> wrote:
> On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index 39b0009253c2..ac3964a9f5c0 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -9074,7 +9075,12 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>> if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
>> netdev->features |= NETIF_F_NTUPLE;
>> if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
>> - netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
>> + netdev->features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
>
> Looks like there might be a typo - I guess this is supposed to still
> OR with the previous features instead of replacing them?
Yes, that was a typo. It looks like I got it right on the i40vf half
of the patch but didn't get it correct here.
Thanks for catching that, I'll have it fixed for when I push it for net-next.
- Alex
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 20:24 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <20160323185006.GL5250@lunn.ch>
Hi,
> My suggestion was that the hardware needs to control the LEDs. You
> have software doing it. You might be able to do this with the PHY
> state machine for link. But activity is never going to be accepted if
> software control.
>
> The LED trigger attached to an LED should be used to configure the
> hardware to drive the LED as wanted.
>
The eth-phy-activity trigger uses the blink_set which I think uses the
hardware acceleration if available. I am not sure how to handles LEDs
which does not have hardware acceleration for this (eth-phy-activity)
trigger.
> The exception to this is when there is no trigger attached, and the
> brightness can set the on/off state, if the hardware supports this.
>
> Andrew
^ permalink raw reply
* CISCO IP PHONES AND CPU's
From: Laison, Computech, Inc <support @ 2016-03-23 18:45 UTC (permalink / raw)
To: Recipients
Hi,
Our Stock list.
Brand NEW
96 x Cisco 7900 IP Phone
87 x Unified IP Phone 6900
12 x Unified IP Phone 8900
76 x Unified IP Phone 9900
55 x Unified IP Phone 8800
67 x Cisco 1921
67 x Cisco 1941
56 x Cisco CP-7961G 7961G
34 x Cisco CP-7971G-GE 7971G
19 x Cisco Unified IP Conference Station 7937G Model: 7937 CP-7937G
20 x Cisco CP-7975G 7975G
Brand New Sealed :
23 x CISCO1921-SEC/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Security Bundle w/SEC license PAK
45 x CISCO1921/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Modular Router, 2 GE, 2 EHWIC slots, 512DRAM, IP Base
(1) WS-C4500X-16SFP+
Serial number: JAE183501L3
US$2600
(1) WS-C3850-48PW-S
Serial number: FCW1823C0EW
US$2650
(1) WS-X6908-10G-2T
Serial number: SAL1620CKUB
US$3650
(1) ASR1000-ESP10
Serial number: JAE181306C3
US$3800
(1) AIR-CT5508-250-K9 (this is new but box is open!)
Serial number: FCW1521L038
US$4000
CPUs part number below
89 x X5650
975 x X5660
150 x X5680
265 x X5690
Kindly make your price offers.
Sincerely
Barbara Johnson
Laison Computech
210 N Scoring Ave,
Rialto California, 92376
Tel: +1-657-232-7047
Fax: +1-347-214-0478
Email: sales@laisoncomputertech.us
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Edward Cree @ 2016-03-23 21:05 UTC (permalink / raw)
To: Alexander Duyck, Or Gerlitz
Cc: Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <CAKgT0Uei6YrWRPaov3LAFrOima=o4+V4mQcFih-rHZ+w-x4hwA@mail.gmail.com>
On 23/03/16 18:06, Alexander Duyck wrote:
> On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
>> My belief is that my way is (in the long run) simpler: ultimately it gets
>> rid of per-protocol GSO callbacks entirely. Every header gets written
>> correctly* when the packet initially traverses the stack, and then at
>> transmit time you either hand that off to TSO, or do the equivalent thing
>> in software: segment at the TCP layer while treating everything above it
>> as an opaque pseudo-L2 header.
> I'm pretty sure that isn't a safe approach to take. With GSO we are
> holding off until we are about to hand the packets off to the device
> before we segment it. If we do it earlier it will lead to more issues
> as you could have the packet route off to somewhere you were not
> expecting and having it already "soft segmented" could lead to issues
> where the packet would no longer be coherent.
Ah, yes, that is a potential problem. And now that I think about it, we
might not know what the MSS is until segmentation time, either, even if
we did know for sure we would want to segment.
So my approach doesn't work after all, the superframe has to be coherent
when it traverses the stack.
>> You mean not supported by offloads, right? We can still _create_ a nested
>> tunnel device, it just won't use hardware offloads. And in the long run,
>> if we can make tunnel-agnostic TSO as I'm proposing, we should be able to
>> support it for nested tunnels too (the giant L2 header just gets more giant).
> Right. Basically what will currently happen is that if you were to
> encapsulate an ipip tunnel inside of a VXLAN for instance the GSO
> would kick in before the VXLAN tunnel has even added the outer headers
> because the VXLAN netdev does not advertise NETIF_F_GSO_IPIP. That is
> the kind of thing we want to have happen though anyway since a
> physical device wouldn't know how to deal with such a scenario anyway.
I disagree. Surely we should be able to "soft segment" the packet just
before we give it to the physical device, and then tell it to do dumb copying
of both the VXLAN and IPIP headers? At this point, we don't have the problem
you identified above, because we've arrived at the device now.
So we can chase through some per-protocol callbacks to shorten all the outer
lengths and adjust all the outer checksums, then hand it to the device for
TSO. The device is treating the extra headers as an opaque blob, so it
doesn't know or care whether it's one layer of encapsulation or forty-two.
>> I think an L2 header extension is a better semantic match for what's
>> happening (the tunnel is an L2 device and we're sending L3 packets over
>> it). But why does it matter? Are you giving the hardware the L2 and
>> L3 headers in separate DMA descriptors or something? The way I see it,
>> all hardware needs to be told is "where to start TSO from", and how it
>> thinks of the stuff before that doesn't matter, because it's not
>> supposed to touch it anyway.
> The problem is setups like VFs where they want to know where the
> network header starts so they know where to insert a VLAN tag. Most
> hardware supports IP options or IPv6 extension headers. What I am
> doing is exploiting that in the case of the Intel hardware by telling
> it the IP header is the outer IP header and the transport header is
> the inner transport header. Then all I have to deal with is the fact
> that hardware will try to compute the entire IPv4 header checksum over
> the range so I cancel that out by seeding the IP checksum with the
> lco_csum added to the inner pseudo-header checksum.
Ok, it sounds like the interface to Intel hardware is just Very Different
to Solarflare hardware on this point: we don't tell our hardware anything
about where the various headers start, it just parses them to figure it
out. (And for new-style TSO we'd tell it where the TCP header starts, as
I described before.)
But this sounds like a driver-level thing: you have to undo some of what
your hardware will do because you're having to lie to it about what you're
giving it. So that all happens in the driver, the stack's GSO code isn't
affected. In which case I'm much less bothered by this; I thought it was
an assumption you were baking into the stack. As far as I'm concerned you
can do whatever ugly hacks you like in Intel drivers, as long as I don't
have to do them in sfc ;)
Although, why is your device computing the IPv4 header checksum? Those
aren't supposed to be offloaded, the stack always already filled them in
in software. (I think sfc makes the same mistake actually.) Is there not
a way for you to tell your device to skip IP header checksum offload?
Then you wouldn't have this problem in the first place, and you could tell
it the IP header was as big as you like without having to seed it with
this correction value.
>> Like I say, I'm assuming we'll start setting DF on outer frames.
>> Besides, it doesn't matter what happens "in transit" - as long as the
>> outer headers aren't touched by the transmitting NIC, the network can
>> do what it likes to them afterwards, without it breaking our TSO code.
> The IP ID bit is going to be something where I don't want to break
> things. One of the things I have seen is there ends up being a number
> of occasions where VXLAN gets fragmented due to incorrectly configured
> MTU. I would rather not have it get completely screwed up when this
> occurs. A performance hit for fragmenting is one thing. Having
> people start complaining because previously working tunnels suddenly
> stop functioning is another. The fact is the whole point of VXLAN is
> to support sending the tunneled frames between data centers. With the
> DF bit set on a tunnel header we end up making it so that frames get
> dropped instead of fragmented which would be a change of behavior.
I agree this isn't something we can do silently. But we _can_ make it a
condition for enabling gso-partial. And I think it's a necessary
condition for truly generic TSO. Sure, your 'L3 extension header' works
fine for a single tunnel. But if you nest tunnels, you now need to
update the outer _and_ middle IP IDs, and you can't do that because you
only have one L3 header pointer.
OTOH, for a single tunnel I think we could implement your 'L3 extension
header' trick in firmware, by making it always parse the outer packet up
to the outer L3 header and increment the IP ID in that. So I could live
with that approach if necessary.
> Yes, I am very confident of that. For Intel hardware the outer VLAN
> tag would be the one inserted via software, the inner VLAN tag is the
> one inserted via hardware.
That's really weird; why does it do that?
For sfc, the only reason we do hardware VLANs at all is to transparently
tag a VF (or non-primary PF) that's being passed through into an
(untrusted) VM. For that use case you'd always want to insert the outer
VLAN tag, because otherwise the VM can escape the isolation by inserting
a different VLAN tag in software.
>> _However_, if we don't need to update the IP IDs, then we can just take
>> the offset of the inner L4 header, and it doesn't make any difference
>> whether you choose to think of the stuff before that as L2 + giant L3
>> or as giant L2 + normal L3, because it's not part of the OS->NIC
>> interface (which is just "L4 starts here"). If your NIC needs to be
>> told where the outer L3 starts as well, then, I guess that's just a
>> wart you need in your drivers. You have skb->network_header, so that
>> shouldn't be difficult - that will always point to the outer one.
> Right. That is kind of what I was going for with this setup. The
> only issue is that the VXLAN tunnels not setting the DF bit kind of
> get in the way of the giant L3 approach.
On the contrary; your giant L3 approach is exactly what solves this case
(for non-nested tunnels) - the hardware has the outer IP and inner TCP
header offsets, which are exactly the two headers it needs to alter.
And if the hardware is sensible, it won't try to re-checksum the whole
giant L3 header, it'll just decrement the (outer) IP checksum to account
for incrementing the (outer) IP ID. If the hardware isn't sensible,
then you have to play games like you are doing in the Intel drivers ;)
> Dealing with the outer header needing the DF bit is something I have
> left unaddressed at this point. The question would be what is the
> correct approach to take for all this. I know RFC2003 for IPv4 in
> IPv4 says you must set the DF bit if the inner header has the DF bit
> set. I'm just wondering if we can apply the same type of logic to GRE
> and UDP tunnels.
I wonder if _not_ setting the DF bit in the outer header can harm TCP
congestion control at all? If so, then we'd pretty much have to set it
in that case.
> My concern is that I am not sure how much value there is to add with
> this type of code if the hardware is parsing headers. In the case of
> most of the Intel parts you specify offsets for the network and
> transport headers so it gives you some degree of flexibility. If
> however the hardware parses headers it becomes problematic as we can
> only support protocols that can be parsed by the hardware.
Solarflare parts will _normally_ parse headers to get that information.
But, when doing TSO, we do get a chance to specify some extra
information, in the TSO option descriptor. Enough of the datapath is
under firmware control that that should be enough; as long as the outer
frame is IP over Ethernet, the hardware will parse that fine, and we
*should* be able to make it just accept that it doesn't know what's
going on between that and the start of the TCP header. And, it
shouldn't matter that the hardware can parse some types of tunnel
headers, because we'll just tell it to ignore that.
Of course, that means changing the firmware; luckily we haven't got any
parts in the wild doing tunnel offloads yet, so we still have a chance
to do that without needing driver code to work around our past
mistakes...
But this stuff does definitely add value for us, it means we could TSO
any tunnel type whatsoever; even nested tunnels as long as only the
outermost IP ID needs to change.
-Ed
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Andrew Lunn @ 2016-03-23 21:10 UTC (permalink / raw)
To: Vishal Thanki; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <CAC3a_SCD6aMF=triCQDJrW9GQ3HOMMH9q_J9GEw56qrnwgFXKA@mail.gmail.com>
On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
> Hi,
>
> > My suggestion was that the hardware needs to control the LEDs. You
> > have software doing it. You might be able to do this with the PHY
> > state machine for link. But activity is never going to be accepted if
> > software control.
> >
> > The LED trigger attached to an LED should be used to configure the
> > hardware to drive the LED as wanted.
> >
>
> The eth-phy-activity trigger uses the blink_set which I think uses the
> hardware acceleration if available. I am not sure how to handles LEDs
> which does not have hardware acceleration for this (eth-phy-activity)
> trigger.
We want the LED to blink on activity, real packets coming in and
out. The PHY can do this, so let the PHY control the LED. In this
case, the trigger is just mechanism for the user to say what the LED
should be used for. The trigger is not itself controlling the LED, it
has no idea about packets coming and going.
Andrew
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Eric Dumazet @ 2016-03-23 21:23 UTC (permalink / raw)
To: Petri Gynther; +Cc: netdev, davem, f.fainelli, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
> 1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
> 2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
> 3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 6746fd0..9990582 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
> struct enet_cb *tx_cb_ptr;
> struct netdev_queue *txq;
> unsigned int pkts_compl = 0;
> + unsigned int bytes_compl = 0;
> unsigned int c_index;
> unsigned int txbds_ready;
> unsigned int txbds_processed = 0;
> @@ -1193,16 +1194,14 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
> if (tx_cb_ptr->skb) {
> pkts_compl++;
> - dev->stats.tx_packets++;
> - dev->stats.tx_bytes += tx_cb_ptr->skb->len;
> + bytes_compl += tx_cb_ptr->skb->len -
> + (priv->desc_64b_en ? 64 : 0);
Please do not use this 64 scalar, prefer sizeof(struct status_64)
I know this driver uses skb_pull(skb, 64); len -= 64; in
bcmgenet_desc_rx(), but I would prefer not adding magic values.
Thanks.
^ permalink raw reply
* Re: [PATCH net 2/2] net: bcmgenet: fix skb_len in bcmgenet_xmit_single()
From: Eric Dumazet @ 2016-03-23 21:24 UTC (permalink / raw)
To: Petri Gynther; +Cc: netdev, davem, f.fainelli, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-2-git-send-email-pgynther@google.com>
On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
> skb_len needs to be skb_headlen(skb) in bcmgenet_xmit_single().
>
> Fragmented skbs can have only Ethernet + IP + TCP headers (14+20+20=54 bytes)
> in the linear buffer, followed by the rest in fragments. Bumping skb_len to
> ETH_ZLEN would be incorrect for this case, as it would introduce garbage
> between TCP header and the fragment data.
>
> This also works with regular/non-fragmented small packets < ETH_ZLEN bytes.
> Successfully tested this on GENETv3 with 42-byte ARP frames.
>
> For testing, I used:
> ethtool -K eth0 tx-checksum-ipv4 off
> ethtool -K eth0 tx-checksum-ipv6 off
> echo 0 > /proc/sys/net/ipv4/tcp_timestamps
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 9990582..00da239 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1298,7 +1298,7 @@ static int bcmgenet_xmit_single(struct net_device *dev,
>
> tx_cb_ptr->skb = skb;
>
> - skb_len = skb_headlen(skb) < ETH_ZLEN ? ETH_ZLEN : skb_headlen(skb);
> + skb_len = skb_headlen(skb);
>
> mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
> ret = dma_mapping_error(kdev, mapping);
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* CISCO IP PHONES AND CPU's
From: Laison, Computech, Inc <support @ 2016-03-23 19:12 UTC (permalink / raw)
To: Recipients
Hi,
Our Stock list.
Brand NEW
96 x Cisco 7900 IP Phone
87 x Unified IP Phone 6900
12 x Unified IP Phone 8900
76 x Unified IP Phone 9900
55 x Unified IP Phone 8800
67 x Cisco 1921
67 x Cisco 1941
56 x Cisco CP-7961G 7961G
34 x Cisco CP-7971G-GE 7971G
19 x Cisco Unified IP Conference Station 7937G Model: 7937 CP-7937G
20 x Cisco CP-7975G 7975G
Brand New Sealed :
23 x CISCO1921-SEC/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Security Bundle w/SEC license PAK
45 x CISCO1921/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Modular Router, 2 GE, 2 EHWIC slots, 512DRAM, IP Base
(1) WS-C4500X-16SFP+
Serial number: JAE183501L3
US$2600
(1) WS-C3850-48PW-S
Serial number: FCW1823C0EW
US$2650
(1) WS-X6908-10G-2T
Serial number: SAL1620CKUB
US$3650
(1) ASR1000-ESP10
Serial number: JAE181306C3
US$3800
(1) AIR-CT5508-250-K9 (this is new but box is open!)
Serial number: FCW1521L038
US$4000
CPUs part number below
89 x X5650
975 x X5660
150 x X5680
265 x X5690
Kindly make your price offers.
Sincerely
Barbara Johnson
Laison Computech
210 N Scoring Ave,
Rialto California, 92376
Tel: +1-657-232-7047
Fax: +1-347-214-0478
Email: sales@laisoncomputertech.us
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 21:24 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <20160323211010.GF19953@lunn.ch>
On Wed, Mar 23, 2016 at 10:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
>> Hi,
>>
>> > My suggestion was that the hardware needs to control the LEDs. You
>> > have software doing it. You might be able to do this with the PHY
>> > state machine for link. But activity is never going to be accepted if
>> > software control.
>> >
>> > The LED trigger attached to an LED should be used to configure the
>> > hardware to drive the LED as wanted.
>> >
>>
>> The eth-phy-activity trigger uses the blink_set which I think uses the
>> hardware acceleration if available. I am not sure how to handles LEDs
>> which does not have hardware acceleration for this (eth-phy-activity)
>> trigger.
>
> We want the LED to blink on activity, real packets coming in and
> out. The PHY can do this, so let the PHY control the LED. In this
> case, the trigger is just mechanism for the user to say what the LED
> should be used for. The trigger is not itself controlling the LED, it
> has no idea about packets coming and going.
>
Yes, I understand that. But PHY can only control the LEDs attached to
it directly. The at803x led driver configures the PHY to blink the
activity LED based on traffic but I think it is not possible for PHY
to control other LEDs in system, for example some other LEDs in system
controlled only via GPIO. In such cases, putting PHY activity trigger
on the GPIO LEDs would not make sense. Correct me if I am wrong.
Vishal
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 21:47 UTC (permalink / raw)
To: Eric Dumazet, Petri Gynther; +Cc: netdev, davem, jaedon.shin, edumazet
In-Reply-To: <1458768237.10868.46.camel@edumazet-glaptop3.roam.corp.google.com>
On March 23, 2016 2:23:57 PM PDT, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
>> 1. Add bytes_compl local variable to collect transmitted bytes
>> - dev->stats updates can then be moved outside the while-loop
>> - bytes_compl is also needed for future BQL support
>> 2. When bcmgenet device uses Tx checksum offload, each transmitted
>> skb has an extra 64-byte header prepended to it. This needs to
>> be deducted when counting transmitted bytes.
>> 3. skb->len covers the entire length of skb, whether it is linear or
>> fragmented. Thus, when we clean the fragments, do not increase
>> transmitted bytes.
>>
>> Signed-off-by: Petri Gynther <pgynther@google.com>
>> ---
>> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index 6746fd0..9990582 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -1171,6 +1171,7 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
>> struct enet_cb *tx_cb_ptr;
>> struct netdev_queue *txq;
>> unsigned int pkts_compl = 0;
>> + unsigned int bytes_compl = 0;
>> unsigned int c_index;
>> unsigned int txbds_ready;
>> unsigned int txbds_processed = 0;
>> @@ -1193,16 +1194,14 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
>> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
>> if (tx_cb_ptr->skb) {
>> pkts_compl++;
>> - dev->stats.tx_packets++;
>> - dev->stats.tx_bytes += tx_cb_ptr->skb->len;
>> + bytes_compl += tx_cb_ptr->skb->len -
>> + (priv->desc_64b_en ? 64 : 0);
>
>Please do not use this 64 scalar, prefer sizeof(struct status_64)
>
>I know this driver uses skb_pull(skb, 64); len -= 64; in
>bcmgenet_desc_rx(), but I would prefer not adding magic values.
Agreed, and just so we are more robust and ready to add BQL in the features please store this information in skb->cb, in the same spirit as what this does:
https://github.com/ffainelli/linux/commits/genet-bql
Thanks.
--
Florian
^ permalink raw reply
* [GIT PULL] target for-next-merge updates for v4.6-rc1
From: Nicholas A. Bellinger @ 2016-03-23 21:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: target-devel, Linux-netdev, David Miller, Hariprasad Shenai,
Varun Prakash, swise, indranil, kxie
Hello Linus,
Here are the target-pending for-next-merge updates for v4.6-rc1.
Please go ahead and pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next-merge
You'll hit one minor merge conflict in t4fw_api.h with net-next as
reported by SFR:
https://lkml.org/lkml/2016/2/29/34
This series contains cxgb4 driver prerequisites for supporting iscsi
segmentation offload (ISO), that will be utilized for a number of future
v4.7 developments in iscsi-target for supporting generic hw offloads.
Thank you,
--nab
Varun Prakash (12):
cxgb4: add new ULD type CXGB4_ULD_ISCSIT
cxgb4: allocate resources for CXGB4_ULD_ISCSIT
cxgb4: large receive offload support
cxgb4, iw_cxgb4: move definitions to common header file
cxgb4, iw_cxgb4, cxgb4i: remove duplicate definitions
cxgb4, cxgb4i: move struct cpl_rx_data_ddp definition
cxgb4: add definitions for iSCSI target ULD
cxgb4: update struct cxgb4_lld_info definition
cxgb4: move VLAN_NONE macro definition
cxgb4, iw_cxgb4: move delayed ack macro definitions
cxgb4: add iSCSI DDP page pod manager
cxgb4: update Kconfig and Makefile
drivers/infiniband/hw/cxgb4/t4fw_ri_api.h | 99 -----
drivers/net/ethernet/chelsio/Kconfig | 11 +
drivers/net/ethernet/chelsio/cxgb4/Makefile | 1 +
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 27 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 34 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 97 ++++-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.c | 464 +++++++++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.h | 310 ++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h | 11 +
drivers/net/ethernet/chelsio/cxgb4/l2t.c | 2 -
drivers/net/ethernet/chelsio/cxgb4/l2t.h | 2 +
drivers/net/ethernet/chelsio/cxgb4/sge.c | 13 +-
drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 217 ++++++++++
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 7 +
drivers/scsi/cxgbi/cxgb4i/cxgb4i.h | 17 -
15 files changed, 1174 insertions(+), 138 deletions(-)
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.c
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.h
^ permalink raw reply
* [PATCH net-next] hv_netvsc: Fix the order of num_sc_offered decrement
From: Haiyang Zhang @ 2016-03-23 21:54 UTC (permalink / raw)
To: davem, netdev; +Cc: olaf, haiyangz, driverdev-devel, linux-kernel
Reorder the code in netvsc_sc_open(), so num_sc_offered is only decremented
after vmbus_open() is called. This avoid pontential race of removing device
before all channels are setup.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/net/hyperv/rndis_filter.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index d5a54da..c4e1e04 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -986,12 +986,6 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
nvscdev = hv_get_drvdata(new_sc->primary_channel->device_obj);
- spin_lock_irqsave(&nvscdev->sc_lock, flags);
- nvscdev->num_sc_offered--;
- spin_unlock_irqrestore(&nvscdev->sc_lock, flags);
- if (nvscdev->num_sc_offered == 0)
- complete(&nvscdev->channel_init_wait);
-
if (chn_index >= nvscdev->num_chn)
return;
@@ -1004,6 +998,12 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
if (ret == 0)
nvscdev->chn_table[chn_index] = new_sc;
+
+ spin_lock_irqsave(&nvscdev->sc_lock, flags);
+ nvscdev->num_sc_offered--;
+ spin_unlock_irqrestore(&nvscdev->sc_lock, flags);
+ if (nvscdev->num_sc_offered == 0)
+ complete(&nvscdev->channel_init_wait);
}
int rndis_filter_device_add(struct hv_device *dev,
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 22:02 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On March 23, 2016 1:01:28 PM PDT, Petri Gynther <pgynther@google.com> wrote:
>1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
>2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
>3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
>Signed-off-by: Petri Gynther <pgynther@google.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
--
Florian
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 22:09 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On March 23, 2016 1:01:28 PM PDT, Petri Gynther <pgynther@google.com> wrote:
>1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
>2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
>3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
>Signed-off-by: Petri Gynther <pgynther@google.com>
Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>index 6746fd0..9990582 100644
>--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>@@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct
>net_device *dev,
> struct enet_cb *tx_cb_ptr;
> struct netdev_queue *txq;
> unsigned int pkts_compl = 0;
>+ unsigned int bytes_compl = 0;
> unsigned int c_index;
> unsigned int txbds_ready;
> unsigned int txbds_processed = 0;
>@@ -1193,16 +1194,14 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
> if (tx_cb_ptr->skb) {
> pkts_compl++;
>- dev->stats.tx_packets++;
>- dev->stats.tx_bytes += tx_cb_ptr->skb->len;
>+ bytes_compl += tx_cb_ptr->skb->len -
>+ (priv->desc_64b_en ? 64 : 0);
> dma_unmap_single(&dev->dev,
> dma_unmap_addr(tx_cb_ptr, dma_addr),
> dma_unmap_len(tx_cb_ptr, dma_len),
> DMA_TO_DEVICE);
> bcmgenet_free_cb(tx_cb_ptr);
> } else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) {
>- dev->stats.tx_bytes +=
>- dma_unmap_len(tx_cb_ptr, dma_len);
> dma_unmap_page(&dev->dev,
> dma_unmap_addr(tx_cb_ptr, dma_addr),
> dma_unmap_len(tx_cb_ptr, dma_len),
>@@ -1220,6 +1219,9 @@ static unsigned int __bcmgenet_tx_reclaim(struct
>net_device *dev,
> ring->free_bds += txbds_processed;
> ring->c_index = (ring->c_index + txbds_processed) & DMA_C_INDEX_MASK;
>
>+ dev->stats.tx_packets += pkts_compl;
>+ dev->stats.tx_bytes += bytes_compl;
>+
> if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
> txq = netdev_get_tx_queue(dev, ring->queue);
> if (netif_tx_queue_stopped(txq))
--
Florian
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 22:36 UTC (permalink / raw)
To: Edward Cree
Cc: Or Gerlitz, Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F30523.8080108@solarflare.com>
On Wed, Mar 23, 2016 at 2:05 PM, Edward Cree <ecree@solarflare.com> wrote:
> On 23/03/16 18:06, Alexander Duyck wrote:
>> On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
>>> My belief is that my way is (in the long run) simpler: ultimately it gets
>>> rid of per-protocol GSO callbacks entirely. Every header gets written
>>> correctly* when the packet initially traverses the stack, and then at
>>> transmit time you either hand that off to TSO, or do the equivalent thing
>>> in software: segment at the TCP layer while treating everything above it
>>> as an opaque pseudo-L2 header.
>> I'm pretty sure that isn't a safe approach to take. With GSO we are
>> holding off until we are about to hand the packets off to the device
>> before we segment it. If we do it earlier it will lead to more issues
>> as you could have the packet route off to somewhere you were not
>> expecting and having it already "soft segmented" could lead to issues
>> where the packet would no longer be coherent.
> Ah, yes, that is a potential problem. And now that I think about it, we
> might not know what the MSS is until segmentation time, either, even if
> we did know for sure we would want to segment.
> So my approach doesn't work after all, the superframe has to be coherent
> when it traverses the stack.
Right.
>>> You mean not supported by offloads, right? We can still _create_ a nested
>>> tunnel device, it just won't use hardware offloads. And in the long run,
>>> if we can make tunnel-agnostic TSO as I'm proposing, we should be able to
>>> support it for nested tunnels too (the giant L2 header just gets more giant).
>> Right. Basically what will currently happen is that if you were to
>> encapsulate an ipip tunnel inside of a VXLAN for instance the GSO
>> would kick in before the VXLAN tunnel has even added the outer headers
>> because the VXLAN netdev does not advertise NETIF_F_GSO_IPIP. That is
>> the kind of thing we want to have happen though anyway since a
>> physical device wouldn't know how to deal with such a scenario anyway.
> I disagree. Surely we should be able to "soft segment" the packet just
> before we give it to the physical device, and then tell it to do dumb copying
> of both the VXLAN and IPIP headers? At this point, we don't have the problem
> you identified above, because we've arrived at the device now.
One issue here is that all levels of IP headers would have to have the
DF bit set. I don't think that happens right now.
> So we can chase through some per-protocol callbacks to shorten all the outer
> lengths and adjust all the outer checksums, then hand it to the device for
> TSO. The device is treating the extra headers as an opaque blob, so it
> doesn't know or care whether it's one layer of encapsulation or forty-two.
So if we do pure software offloads this is doable. However the GSO
flags are meant to have hardware feature equivalents. The problem is
if you combine an IPIP and VXLAN header how do you know what header is
what and which order things are in, and what is the likelihood of
having a device that would get things right when dealing with 3 levels
of IP headers. This is one of the reasons why we don't support
multiple levels of tunnels in the GSO code. GSO is just meant to be a
fall-back for hardware offloads.
>>> I think an L2 header extension is a better semantic match for what's
>>> happening (the tunnel is an L2 device and we're sending L3 packets over
>>> it). But why does it matter? Are you giving the hardware the L2 and
>>> L3 headers in separate DMA descriptors or something? The way I see it,
>>> all hardware needs to be told is "where to start TSO from", and how it
>>> thinks of the stuff before that doesn't matter, because it's not
>>> supposed to touch it anyway.
>> The problem is setups like VFs where they want to know where the
>> network header starts so they know where to insert a VLAN tag. Most
>> hardware supports IP options or IPv6 extension headers. What I am
>> doing is exploiting that in the case of the Intel hardware by telling
>> it the IP header is the outer IP header and the transport header is
>> the inner transport header. Then all I have to deal with is the fact
>> that hardware will try to compute the entire IPv4 header checksum over
>> the range so I cancel that out by seeding the IP checksum with the
>> lco_csum added to the inner pseudo-header checksum.
> Ok, it sounds like the interface to Intel hardware is just Very Different
> to Solarflare hardware on this point: we don't tell our hardware anything
> about where the various headers start, it just parses them to figure it
> out. (And for new-style TSO we'd tell it where the TCP header starts, as
> I described before.)
That is kind of what I figured. So does that mean for IPv6 you guys
are parsing through extension headers? I believe that is one of the
reasons why Intel did things the way they did is to avoid having to
parse through any IPv4 options or IPv6 extension headers.
> But this sounds like a driver-level thing: you have to undo some of what
> your hardware will do because you're having to lie to it about what you're
> giving it. So that all happens in the driver, the stack's GSO code isn't
> affected. In which case I'm much less bothered by this; I thought it was
> an assumption you were baking into the stack. As far as I'm concerned you
> can do whatever ugly hacks you like in Intel drivers, as long as I don't
> have to do them in sfc ;)
The thing is I have to have the hardware recompute the outer IPv4
checksum because it is updating the ID field. If I were able to leave
the IP ID static then I wouldn't need to update the checksum. The
only bit that makes it ugly is the fact that the hardware is being
misled so it thinks the IPv4 header has 50 bytes of options when those
actually represent the outer L4 through to the inner L3 headers.
> Although, why is your device computing the IPv4 header checksum? Those
> aren't supposed to be offloaded, the stack always already filled them in
> in software. (I think sfc makes the same mistake actually.) Is there not
> a way for you to tell your device to skip IP header checksum offload?
> Then you wouldn't have this problem in the first place, and you could tell
> it the IP header was as big as you like without having to seed it with
> this correction value.
It is because we are rewriting the IP ID. We cannot have a static
checksum if the IP ID is getting updated.
>>> Like I say, I'm assuming we'll start setting DF on outer frames.
>>> Besides, it doesn't matter what happens "in transit" - as long as the
>>> outer headers aren't touched by the transmitting NIC, the network can
>>> do what it likes to them afterwards, without it breaking our TSO code.
>> The IP ID bit is going to be something where I don't want to break
>> things. One of the things I have seen is there ends up being a number
>> of occasions where VXLAN gets fragmented due to incorrectly configured
>> MTU. I would rather not have it get completely screwed up when this
>> occurs. A performance hit for fragmenting is one thing. Having
>> people start complaining because previously working tunnels suddenly
>> stop functioning is another. The fact is the whole point of VXLAN is
>> to support sending the tunneled frames between data centers. With the
>> DF bit set on a tunnel header we end up making it so that frames get
>> dropped instead of fragmented which would be a change of behavior.
> I agree this isn't something we can do silently. But we _can_ make it a
> condition for enabling gso-partial. And I think it's a necessary
> condition for truly generic TSO. Sure, your 'L3 extension header' works
> fine for a single tunnel. But if you nest tunnels, you now need to
> update the outer _and_ middle IP IDs, and you can't do that because you
> only have one L3 header pointer.
This is getting away from the 'less is more' concept. If we are doing
multiple levels of tunnels we have already made things far too
complicated and it is unlikely hardware will ever support anything
like that.
> OTOH, for a single tunnel I think we could implement your 'L3 extension
> header' trick in firmware, by making it always parse the outer packet up
> to the outer L3 header and increment the IP ID in that. So I could live
> with that approach if necessary.
Good to hear.
>> Yes, I am very confident of that. For Intel hardware the outer VLAN
>> tag would be the one inserted via software, the inner VLAN tag is the
>> one inserted via hardware.
> That's really weird; why does it do that?
> For sfc, the only reason we do hardware VLANs at all is to transparently
> tag a VF (or non-primary PF) that's being passed through into an
> (untrusted) VM. For that use case you'd always want to insert the outer
> VLAN tag, because otherwise the VM can escape the isolation by inserting
> a different VLAN tag in software.
The problem is I think the feature was implemented long before any of
the SR-IOV stuff was added and it has been carried that way through
the igb and ixgbe drivers. I haven't looked at i40e but it doesn't
report doing STAG so I don't know how it handles QinQ.
>>> _However_, if we don't need to update the IP IDs, then we can just take
>>> the offset of the inner L4 header, and it doesn't make any difference
>>> whether you choose to think of the stuff before that as L2 + giant L3
>>> or as giant L2 + normal L3, because it's not part of the OS->NIC
>>> interface (which is just "L4 starts here"). If your NIC needs to be
>>> told where the outer L3 starts as well, then, I guess that's just a
>>> wart you need in your drivers. You have skb->network_header, so that
>>> shouldn't be difficult - that will always point to the outer one.
>> Right. That is kind of what I was going for with this setup. The
>> only issue is that the VXLAN tunnels not setting the DF bit kind of
>> get in the way of the giant L3 approach.
Sorry meant giant L2 approach here, not L3.
> On the contrary; your giant L3 approach is exactly what solves this case
> (for non-nested tunnels) - the hardware has the outer IP and inner TCP
> header offsets, which are exactly the two headers it needs to alter.
> And if the hardware is sensible, it won't try to re-checksum the whole
> giant L3 header, it'll just decrement the (outer) IP checksum to account
> for incrementing the (outer) IP ID. If the hardware isn't sensible,
> then you have to play games like you are doing in the Intel drivers ;)
Agreed.
>> Dealing with the outer header needing the DF bit is something I have
>> left unaddressed at this point. The question would be what is the
>> correct approach to take for all this. I know RFC2003 for IPv4 in
>> IPv4 says you must set the DF bit if the inner header has the DF bit
>> set. I'm just wondering if we can apply the same type of logic to GRE
>> and UDP tunnels.
> I wonder if _not_ setting the DF bit in the outer header can harm TCP
> congestion control at all? If so, then we'd pretty much have to set it
> in that case.
Really the only concern with DF based on RFC 2003 is for path MTU
discovery. What happens right now if you set the VXLAN MTU
incorrectly is you end up taking a performance hit as the tunneled
frames are fragmented and have to be reassembled on the other end. It
ends up being a performance hit due to the fragmentation. I might
need to look into this. It is possible that we are already doing the
wrong stuff anyway with this and might need to look at setting the DF
bit anyway.
>> My concern is that I am not sure how much value there is to add with
>> this type of code if the hardware is parsing headers. In the case of
>> most of the Intel parts you specify offsets for the network and
>> transport headers so it gives you some degree of flexibility. If
>> however the hardware parses headers it becomes problematic as we can
>> only support protocols that can be parsed by the hardware.
> Solarflare parts will _normally_ parse headers to get that information.
> But, when doing TSO, we do get a chance to specify some extra
> information, in the TSO option descriptor. Enough of the datapath is
> under firmware control that that should be enough; as long as the outer
> frame is IP over Ethernet, the hardware will parse that fine, and we
> *should* be able to make it just accept that it doesn't know what's
> going on between that and the start of the TCP header. And, it
> shouldn't matter that the hardware can parse some types of tunnel
> headers, because we'll just tell it to ignore that.
Right. Just skipping the tunnel headers makes it quite a bit easier.
> Of course, that means changing the firmware; luckily we haven't got any
> parts in the wild doing tunnel offloads yet, so we still have a chance
> to do that without needing driver code to work around our past
> mistakes...
>
> But this stuff does definitely add value for us, it means we could TSO
> any tunnel type whatsoever; even nested tunnels as long as only the
> outermost IP ID needs to change.
Right. In your case it sounds like you would have the advantage of
just having to run essentially two counters, one increments the IPv4
ID and the other decrements the IPv4 checksum. Beyond that the outer
headers wouldn't need to change at all.
The only other issue would be determining how the inner pseudo-header
checksum is updated. If you were parsing out header fields from the
IP header previously to generate it you would instead need to update
things so that you could use the partial checksum that is already
stored in the TCP header checksum field.
- Alex
^ permalink raw reply
* [PATCH RFC 0/9] udp: GRO in UDP sockets and fast encap_rcv
From: Tom Herbert @ 2016-03-23 22:36 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team
This patch set does two things:
- Add GRO functions (gro_receive and gro_complete) to UDP sockets.
- Implement "fast receive encapsulation".
Add GRO functions (gro_receive and gro_complete) to UDP sockets. In
udp_gro_receive and udp_gro_complete a socket lookup is done instead of
looking up the port number in udp_offloads. If a socket is found and
there is are gro functions for it then those are called. This feature
allows binding GRO functions to more than just a port number.
Eventually, we will be able to use this technique to allow application
defined GRO for an application protocol by attaching BPF porgrams to UDP
sockets for doing GRO.
Fast receive encapsulation allows encap_rcv to be called for a socket
without taking the reference to the socket. The callee can assume it
was called under rcu_read_lock, but must assume that no reference to the
socket is held (so the socket structures should not be written to and
pointer not saved).
In order to implement these function, no socket reference lookup
functions were added in ipv4/udp.c and ipv6/udp.c. Also, inet_iif and
references to skb_dst() were changed to check that dst is set in skbuf
before derefencing. In the GRO path there is now a UDP socket lookup
performed before dst is set, to the get the device in that case we
simply use skb->dev.
Tested:
Ran various combinations of VXLAN and GUE TCP_STREAM and TCP_RR tests.
DId not see any regressions. perf shows a that the atomic opertions in
taking a reference to UDP sockets in lookup is eliminated for
encapsulation.
Also ran plain UDP_RR. DId not see any regression there.
Tom Herbert (9):
net: Check skb_dst for NULL in inet_iif
udp: Add noreference lookup functions
net: Add fast receive encapsulation
udp: Add GRO functions to UDP socket
udp: Add socket based GRO and fast receive encap to tunnel config
vxlan: change vxlan to use UDP socket GRO
fou: change to use UDP socket GRO and fast rcv encap
geneve: change to use UDP socket GRO
udp: Remove udp_offloads
drivers/net/geneve.c | 29 ++----
drivers/net/vxlan.c | 31 ++----
include/linux/netdevice.h | 17 ----
include/linux/udp.h | 13 ++-
include/net/protocol.h | 3 -
include/net/route.h | 7 +-
include/net/udp.h | 19 +++-
include/net/udp_tunnel.h | 8 ++
include/net/vxlan.h | 1 -
net/ipv4/fou.c | 49 ++++------
net/ipv4/udp.c | 237 +++++++++++++++++++++++++++++-----------------
net/ipv4/udp_offload.c | 113 ++++------------------
net/ipv4/udp_tunnel.c | 3 +
net/ipv6/Makefile | 5 +-
net/ipv6/af_inet6.c | 8 ++
net/ipv6/ip6_offload.c | 2 -
net/ipv6/ip6_offload.h | 3 +-
net/ipv6/udp.c | 166 +++++++++++++++++---------------
net/ipv6/udp_offload.c | 11 ++-
19 files changed, 360 insertions(+), 365 deletions(-)
--
2.8.0.rc2
^ permalink raw reply
* [PATCH RFC 3/9] net: Add fast receive encapsulation
From: Tom Herbert @ 2016-03-23 22:36 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1458772618-845742-1-git-send-email-tom@herbertland.com>
This patch allows fast receive encapsulation processing. A configuration
flag, encap_fast, may be set in a UDP socket. When this flag is set
encap_rcv may be called without taking a reference to the the
encapsulation socket (which is usually unnecessary since the
encapsulation socket is not written to or saved in an skbuff).
In udp.c the logic to handle encapsulated packets is changed. When
receive a packet:
1) Perform a noref socket lookup.
2) If found socket is an encapsulation socket and encap_fast is set
call encap_rcv with taking a reference
3) If further processing is needed, including calling encap_rcv
when encap_fast is not set, the take a reference to the socket
This patch adds udp_encap_rcv_check to check and run encap_rcv.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/linux/udp.h | 5 +-
net/ipv4/udp.c | 165 ++++++++++++++++++++++++++++++----------------------
net/ipv6/udp.c | 91 +++++++++++------------------
3 files changed, 133 insertions(+), 128 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 87c0949..f58213e 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -49,7 +49,8 @@ struct udp_sock {
unsigned int corkflag; /* Cork is required */
__u8 encap_type; /* Is this an Encapsulation socket? */
unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
- no_check6_rx:1;/* Allow zero UDP6 checksums on RX? */
+ no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
+ encap_fast:1; /* Can call encap_rcv wihout ref */
/*
* Following member retains the information to create a UDP header
* when the socket is uncorked.
@@ -98,6 +99,8 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
return udp_sk(sk)->no_check6_rx;
}
+int udp_encap_rcv_check(struct sock *sk, struct sk_buff *skb);
+
#define udp_portaddr_for_each_entry(__sk, node, list) \
hlist_nulls_for_each_entry(__sk, node, list, __sk_common.skc_portaddr_node)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 324d008..cb13ec0 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -678,17 +678,6 @@ struct sock *udp4_lib_lookup(struct net *net, __be32 saddr, __be16 sport,
}
EXPORT_SYMBOL_GPL(udp4_lib_lookup);
-static inline struct sock *__udp4_lib_lookup_skb(struct sk_buff *skb,
- __be16 sport, __be16 dport,
- struct udp_table *udptable)
-{
- const struct iphdr *iph = ip_hdr(skb);
-
- return __udp4_lib_lookup(dev_net(skb_dst(skb)->dev), iph->saddr, sport,
- iph->daddr, dport, inet_iif(skb),
- udptable, skb);
-}
-
static inline struct sock *__udp4_lib_lookup_skb_noref(struct sk_buff *skb,
__be16 sport, __be16 dport,
struct udp_table *udptable)
@@ -1611,9 +1600,65 @@ void udp_encap_enable(void)
EXPORT_SYMBOL(udp_encap_enable);
/* returns:
+ * =0 if skb was successfully passed to the encap
+ * handler or was discarded by it.
+ * >0 if skb should be passed on to UDP.
+ * <0 if skb should be resubmitted as proto -N
+ *
+ * Note that in the success and error cases, the skb is assumed to
+ * have either been requeued or freed.
+ */
+int udp_encap_rcv_check(struct sock *sk, struct sk_buff *skb)
+{
+ struct udp_sock *up = udp_sk(sk);
+ int is_udplite = IS_UDPLITE(sk);
+
+ int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+
+ if (!static_key_false(&udp_encap_needed))
+ return 1;
+ /* This is an encapsulation socket so pass the skb to
+ * the socket's udp_encap_rcv() hook. Otherwise, just
+ * fall through and pass this up the UDP socket.
+ * up->encap_rcv() returns the following value:
+ * =0 if skb was successfully passed to the encap
+ * handler or was discarded by it.
+ * >0 if skb should be passed on to UDP.
+ * <0 if skb should be resubmitted as proto -N
+ */
+
+ /* if we're overly short, let UDP handle it */
+ encap_rcv = ACCESS_ONCE(up->encap_rcv);
+ if (skb->len > sizeof(struct udphdr) && encap_rcv) {
+ int ret;
+
+ /* Verify checksum before giving to encap */
+ if (udp_lib_checksum_complete(skb))
+ goto csum_error;
+
+ ret = encap_rcv(sk, skb);
+ if (ret <= 0) {
+ UDP_INC_STATS_BH(sock_net(sk),
+ UDP_MIB_INDATAGRAMS,
+ is_udplite);
+ return ret;
+ }
+ }
+
+ return 1; /* Continue UDP processing */
+
+csum_error:
+ UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
+ UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+ atomic_inc(&sk->sk_drops);
+ kfree_skb(skb);
+ return 0; /* Dropped */
+}
+EXPORT_SYMBOL(udp_encap_rcv_check);
+
+/* returns:
* -1: error
* 0: success
- * >0: "udp encap" protocol resubmission
*
* Note that in the success and error cases, the skb is assumed to
* have either been requeued or freed.
@@ -1621,8 +1666,8 @@ EXPORT_SYMBOL(udp_encap_enable);
int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
- int rc;
int is_udplite = IS_UDPLITE(sk);
+ int rc;
/*
* Charge it to the socket, dropping if the queue is full.
@@ -1631,41 +1676,6 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
goto drop;
nf_reset(skb);
- if (static_key_false(&udp_encap_needed) && up->encap_type) {
- int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
-
- /*
- * This is an encapsulation socket so pass the skb to
- * the socket's udp_encap_rcv() hook. Otherwise, just
- * fall through and pass this up the UDP socket.
- * up->encap_rcv() returns the following value:
- * =0 if skb was successfully passed to the encap
- * handler or was discarded by it.
- * >0 if skb should be passed on to UDP.
- * <0 if skb should be resubmitted as proto -N
- */
-
- /* if we're overly short, let UDP handle it */
- encap_rcv = ACCESS_ONCE(up->encap_rcv);
- if (skb->len > sizeof(struct udphdr) && encap_rcv) {
- int ret;
-
- /* Verify checksum before giving to encap */
- if (udp_lib_checksum_complete(skb))
- goto csum_error;
-
- ret = encap_rcv(sk, skb);
- if (ret <= 0) {
- UDP_INC_STATS_BH(sock_net(sk),
- UDP_MIB_INDATAGRAMS,
- is_udplite);
- return -ret;
- }
- }
-
- /* FALLTHROUGH -- it's a UDP Packet */
- }
-
/*
* UDP-Lite specific tests, ignored on UDP sockets
*/
@@ -1864,10 +1874,14 @@ static inline int udp4_csum_init(struct sk_buff *skb, struct udphdr *uh,
inet_compute_pseudo);
}
-/*
- * All we need to do is get the socket, and then do a checksum.
+/* Process a received UDP packet. Validate the packet and checksum,
+ * lookup a socket, either receive packet on that socket or call
+ * the specified encapsulation receive function.
+ *
+ * Returns:
+ * 0: packet was received or consumed
+ * <0: "udp encap" protocol resubmission
*/
-
int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
int proto)
{
@@ -1905,26 +1919,20 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
sk = skb_steal_sock(skb);
if (sk) {
struct dst_entry *dst = skb_dst(skb);
- int ret;
if (unlikely(sk->sk_rx_dst != dst))
udp_sk_rx_dst_set(sk, dst);
- ret = udp_queue_rcv_skb(sk, skb);
- sock_put(sk);
- /* a return value > 0 means to resubmit the input, but
- * it wants the return to be -protocol, or 0
- */
- if (ret > 0)
- return -ret;
- return 0;
+ goto have_ref_sock;
}
if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
return __udp4_lib_mcast_deliver(net, skb, uh,
saddr, daddr, udptable, proto);
- sk = __udp4_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
+ rcu_read_lock();
+ /* Don't take socket reference unless we need to */
+ sk = __udp4_lib_lookup_skb_noref(skb, uh->source, uh->dest, udptable);
if (sk) {
int ret;
@@ -1932,15 +1940,34 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
inet_compute_pseudo);
- ret = udp_queue_rcv_skb(sk, skb);
- sock_put(sk);
+ if (udp_sk(sk)->encap_type && udp_sk(sk)->encap_fast) {
+ ret = udp_encap_rcv_check(sk, skb);
+ if (ret <= 0) {
+ rcu_read_unlock();
+ return ret;
+ }
+ }
- /* a return value > 0 means to resubmit the input, but
- * it wants the return to be -protocol, or 0
- */
- if (ret > 0)
- return -ret;
- return 0;
+ /* Okay, need reference for futher processing */
+ sk = udp_get_ref(sk);
+ rcu_read_unlock();
+
+ if (sk) {
+have_ref_sock:
+ if (udp_sk(sk)->encap_type && !udp_sk(sk)->encap_fast) {
+ /* Did not check for encap yet */
+ ret = udp_encap_rcv_check(sk, skb);
+ if (ret <= 0) {
+ sock_put(sk);
+ return ret;
+ }
+ }
+ ret = udp_queue_rcv_skb(sk, skb);
+ sock_put(sk);
+ return ret;
+ }
+ } else {
+ rcu_read_unlock();
}
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 281469c..cbcac8f 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -386,21 +386,6 @@ struct sock *__udp6_lib_lookup(struct net *net,
}
EXPORT_SYMBOL_GPL(__udp6_lib_lookup);
-static struct sock *__udp6_lib_lookup_skb(struct sk_buff *skb,
- __be16 sport, __be16 dport,
- struct udp_table *udptable)
-{
- struct sock *sk;
- const struct ipv6hdr *iph = ipv6_hdr(skb);
-
- sk = skb_steal_sock(skb);
- if (unlikely(sk))
- return sk;
- return __udp6_lib_lookup(dev_net(skb_dst(skb)->dev), &iph->saddr, sport,
- &iph->daddr, dport, inet6_iif(skb),
- udptable, skb);
-}
-
struct sock *udp6_lib_lookup(struct net *net, const struct in6_addr *saddr, __be16 sport,
const struct in6_addr *daddr, __be16 dport, int dif)
{
@@ -676,41 +661,6 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto drop;
- if (static_key_false(&udpv6_encap_needed) && up->encap_type) {
- int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
-
- /*
- * This is an encapsulation socket so pass the skb to
- * the socket's udp_encap_rcv() hook. Otherwise, just
- * fall through and pass this up the UDP socket.
- * up->encap_rcv() returns the following value:
- * =0 if skb was successfully passed to the encap
- * handler or was discarded by it.
- * >0 if skb should be passed on to UDP.
- * <0 if skb should be resubmitted as proto -N
- */
-
- /* if we're overly short, let UDP handle it */
- encap_rcv = ACCESS_ONCE(up->encap_rcv);
- if (skb->len > sizeof(struct udphdr) && encap_rcv) {
- int ret;
-
- /* Verify checksum before giving to encap */
- if (udp_lib_checksum_complete(skb))
- goto csum_error;
-
- ret = encap_rcv(sk, skb);
- if (ret <= 0) {
- UDP_INC_STATS_BH(sock_net(sk),
- UDP_MIB_INDATAGRAMS,
- is_udplite);
- return -ret;
- }
- }
-
- /* FALLTHROUGH -- it's a UDP Packet */
- }
-
/*
* UDP-Lite specific tests, ignored on UDP sockets (see net/ipv4/udp.c).
*/
@@ -944,7 +894,13 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
* check socket cache ... must talk to Alan about his plans
* for sock caches... i'll skip this for now.
*/
- sk = __udp6_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
+ sk = skb_steal_sock(skb);
+ if (unlikely(sk))
+ goto have_ref_sock;
+
+ rcu_read_lock();
+ /* Don't take socket reference unless we need to */
+ sk = __udp6_lib_lookup_skb_noref(skb, uh->source, uh->dest, udptable);
if (sk) {
int ret;
@@ -958,14 +914,33 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
ip6_compute_pseudo);
- ret = udpv6_queue_rcv_skb(sk, skb);
- sock_put(sk);
-
- /* a return value > 0 means to resubmit the input */
- if (ret > 0)
- return ret;
+ if (udp_sk(sk)->encap_type && udp_sk(sk)->encap_fast) {
+ ret = udp_encap_rcv_check(sk, skb);
+ if (ret) {
+ rcu_read_unlock();
+ return ret > 0 ? -ret : 0;
+ }
+ }
- return 0;
+ /* Okay, need reference for futher processing */
+ sk = udp_get_ref(sk);
+ rcu_read_unlock();
+
+ if (sk) {
+have_ref_sock:
+ if (udp_sk(sk)->encap_type && !udp_sk(sk)->encap_fast) {
+ /* Did not check for encap yet */
+ ret = udp_encap_rcv_check(sk, skb);
+ if (ret)
+ goto out;
+ }
+ ret = udpv6_queue_rcv_skb(sk, skb);
+out:
+ sock_put(sk);
+ return ret > 0 ? -ret : 0;
+ }
+ } else {
+ rcu_read_unlock();
}
if (!uh->check) {
--
2.8.0.rc2
^ permalink raw reply related
* [PATCH RFC 4/9] udp: Add GRO functions to UDP socket
From: Tom Herbert @ 2016-03-23 22:36 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1458772618-845742-1-git-send-email-tom@herbertland.com>
This patch adds GRO functions (gro_receive and gro_complete) to UDP
sockets. udp_gro_receive is changed to perfrom a noref socket lookup
on a packet. If a socket is found the related GRO functions are called.
This features obsoletes using UDP offload infrastructure for GRO
(udp_offload). This has the advantage of not being limited to provide
offload on a per port basis, GRO is now applied to whatever individual
UDP sockets are bound to. This also allows the possbility of
"application defined GRO"-- that is we can attach something like
a BPF program to a UDP socket to perfrom GRO on an application
layer protocol.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/linux/udp.h | 8 ++++++++
include/net/udp.h | 7 +++++--
net/ipv4/udp_offload.c | 52 +++++++++++++++++++-------------------------------
net/ipv6/Makefile | 5 +++--
net/ipv6/af_inet6.c | 8 ++++++++
net/ipv6/ip6_offload.c | 2 --
net/ipv6/ip6_offload.h | 3 ++-
net/ipv6/udp_offload.c | 11 ++++++++---
8 files changed, 54 insertions(+), 42 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index f58213e..e905103 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -72,6 +72,14 @@ struct udp_sock {
*/
int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
void (*encap_destroy)(struct sock *sk);
+
+ /* GRO functions for UDP socket */
+ struct sk_buff ** (*gro_receive)(struct sock *sk,
+ struct sk_buff **head,
+ struct sk_buff *skb);
+ int (*gro_complete)(struct sock *sk,
+ struct sk_buff *skb,
+ int nhoff);
};
static inline struct udp_sock *udp_sk(const struct sock *sk)
diff --git a/include/net/udp.h b/include/net/udp.h
index 2a6f7b2..94e0ab6 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -158,9 +158,12 @@ static inline __sum16 udp_v4_check(int len, __be32 saddr,
void udp_set_csum(bool nocheck, struct sk_buff *skb,
__be32 saddr, __be32 daddr, int len);
+typedef struct sock *(*udp_lookup_t)(struct sk_buff *skb, __be16 sport,
+ __be16 dport);
+
struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
- struct udphdr *uh);
-int udp_gro_complete(struct sk_buff *skb, int nhoff);
+ struct udphdr *uh, udp_lookup_t lookup);
+int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
{
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 0e89f80..dce43c9 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -179,6 +179,7 @@ out_unlock:
return segs;
}
+EXPORT_SYMBOL(skb_udp_tunnel_segment);
static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
netdev_features_t features)
@@ -304,13 +305,13 @@ unlock:
EXPORT_SYMBOL(udp_del_offload);
struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
- struct udphdr *uh)
+ struct udphdr *uh, udp_lookup_t lookup)
{
- struct udp_offload_priv *uo_priv;
struct sk_buff *p, **pp = NULL;
struct udphdr *uh2;
unsigned int off = skb_gro_offset(skb);
int flush = 1;
+ struct sock *sk;
if (NAPI_GRO_CB(skb)->udp_mark ||
(skb->ip_summed != CHECKSUM_PARTIAL &&
@@ -322,13 +323,11 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
NAPI_GRO_CB(skb)->udp_mark = 1;
rcu_read_lock();
- uo_priv = rcu_dereference(udp_offload_base);
- for (; uo_priv != NULL; uo_priv = rcu_dereference(uo_priv->next)) {
- if (net_eq(read_pnet(&uo_priv->net), dev_net(skb->dev)) &&
- uo_priv->offload->port == uh->dest &&
- uo_priv->offload->callbacks.gro_receive)
- goto unflush;
- }
+ sk = (*lookup)(skb, uh->source, uh->dest);
+
+ if (sk && udp_sk(sk)->gro_receive)
+ goto unflush;
+
goto out_unlock;
unflush:
@@ -352,9 +351,7 @@ unflush:
skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
- NAPI_GRO_CB(skb)->proto = uo_priv->offload->ipproto;
- pp = uo_priv->offload->callbacks.gro_receive(head, skb,
- uo_priv->offload);
+ pp = udp_sk(sk)->gro_receive(sk, head, skb);
out_unlock:
rcu_read_unlock();
@@ -362,6 +359,7 @@ out:
NAPI_GRO_CB(skb)->flush |= flush;
return pp;
}
+EXPORT_SYMBOL(udp_gro_receive);
static struct sk_buff **udp4_gro_receive(struct sk_buff **head,
struct sk_buff *skb)
@@ -383,39 +381,28 @@ static struct sk_buff **udp4_gro_receive(struct sk_buff **head,
inet_gro_compute_pseudo);
skip:
NAPI_GRO_CB(skb)->is_ipv6 = 0;
- return udp_gro_receive(head, skb, uh);
+ return udp_gro_receive(head, skb, uh, udp4_lib_lookup_skb_noref);
flush:
NAPI_GRO_CB(skb)->flush = 1;
return NULL;
}
-int udp_gro_complete(struct sk_buff *skb, int nhoff)
+int udp_gro_complete(struct sk_buff *skb, int nhoff,
+ udp_lookup_t lookup)
{
- struct udp_offload_priv *uo_priv;
__be16 newlen = htons(skb->len - nhoff);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
int err = -ENOSYS;
+ struct sock *sk;
uh->len = newlen;
rcu_read_lock();
-
- uo_priv = rcu_dereference(udp_offload_base);
- for (; uo_priv != NULL; uo_priv = rcu_dereference(uo_priv->next)) {
- if (net_eq(read_pnet(&uo_priv->net), dev_net(skb->dev)) &&
- uo_priv->offload->port == uh->dest &&
- uo_priv->offload->callbacks.gro_complete)
- break;
- }
-
- if (uo_priv) {
- NAPI_GRO_CB(skb)->proto = uo_priv->offload->ipproto;
- err = uo_priv->offload->callbacks.gro_complete(skb,
- nhoff + sizeof(struct udphdr),
- uo_priv->offload);
- }
-
+ sk = (*lookup)(skb, uh->source, uh->dest);
+ if (sk && udp_sk(sk)->gro_complete)
+ err = udp_sk(sk)->gro_complete(sk, skb,
+ nhoff + sizeof(struct udphdr));
rcu_read_unlock();
if (skb->remcsum_offload)
@@ -426,6 +413,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
return err;
}
+EXPORT_SYMBOL(udp_gro_complete);
static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
{
@@ -440,7 +428,7 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
}
- return udp_gro_complete(skb, nhoff);
+ return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb_noref);
}
static const struct net_offload udpv4_offload = {
diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
index 2fbd90b..5e9d6bf 100644
--- a/net/ipv6/Makefile
+++ b/net/ipv6/Makefile
@@ -8,9 +8,10 @@ ipv6-objs := af_inet6.o anycast.o ip6_output.o ip6_input.o addrconf.o \
addrlabel.o \
route.o ip6_fib.o ipv6_sockglue.o ndisc.o udp.o udplite.o \
raw.o icmp.o mcast.o reassembly.o tcp_ipv6.o ping.o \
- exthdrs.o datagram.o ip6_flowlabel.o inet6_connection_sock.o
+ exthdrs.o datagram.o ip6_flowlabel.o inet6_connection_sock.o \
+ udp_offload.o
-ipv6-offload := ip6_offload.o tcpv6_offload.o udp_offload.o exthdrs_offload.o
+ipv6-offload := ip6_offload.o tcpv6_offload.o exthdrs_offload.o
ipv6-$(CONFIG_SYSCTL) = sysctl_net_ipv6.o
ipv6-$(CONFIG_IPV6_MROUTE) += ip6mr.o
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index b11c37c..9a2fec5 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -64,6 +64,8 @@
#include <asm/uaccess.h>
#include <linux/mroute6.h>
+#include "ip6_offload.h"
+
MODULE_AUTHOR("Cast of dozens");
MODULE_DESCRIPTION("IPv6 protocol stack for Linux");
MODULE_LICENSE("GPL");
@@ -958,6 +960,10 @@ static int __init inet6_init(void)
if (err)
goto udplitev6_fail;
+ err = udpv6_offload_init();
+ if (err)
+ goto udpv6_offload_fail;
+
err = tcpv6_init();
if (err)
goto tcpv6_fail;
@@ -987,6 +993,8 @@ pingv6_fail:
ipv6_packet_fail:
tcpv6_exit();
tcpv6_fail:
+ udpv6_offload_exit();
+udpv6_offload_fail:
udplitev6_exit();
udplitev6_fail:
udpv6_exit();
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index eeca943..d96c5e2 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -312,8 +312,6 @@ static int __init ipv6_offload_init(void)
if (tcpv6_offload_init() < 0)
pr_crit("%s: Cannot add TCP protocol offload\n", __func__);
- if (udp_offload_init() < 0)
- pr_crit("%s: Cannot add UDP protocol offload\n", __func__);
if (ipv6_exthdrs_offload_init() < 0)
pr_crit("%s: Cannot add EXTHDRS protocol offload\n", __func__);
diff --git a/net/ipv6/ip6_offload.h b/net/ipv6/ip6_offload.h
index 2e155c6..96b40e4 100644
--- a/net/ipv6/ip6_offload.h
+++ b/net/ipv6/ip6_offload.h
@@ -12,7 +12,8 @@
#define __ip6_offload_h
int ipv6_exthdrs_offload_init(void);
-int udp_offload_init(void);
+int udpv6_offload_init(void);
+int udpv6_offload_exit(void);
int tcpv6_offload_init(void);
#endif
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 2b0fbe6..4a822f6 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -153,7 +153,7 @@ static struct sk_buff **udp6_gro_receive(struct sk_buff **head,
skip:
NAPI_GRO_CB(skb)->is_ipv6 = 1;
- return udp_gro_receive(head, skb, uh);
+ return udp_gro_receive(head, skb, uh, udp6_lib_lookup_skb_noref);
flush:
NAPI_GRO_CB(skb)->flush = 1;
@@ -173,7 +173,7 @@ static int udp6_gro_complete(struct sk_buff *skb, int nhoff)
skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
}
- return udp_gro_complete(skb, nhoff);
+ return udp_gro_complete(skb, nhoff, udp6_lib_lookup_skb_noref);
}
static const struct net_offload udpv6_offload = {
@@ -184,7 +184,12 @@ static const struct net_offload udpv6_offload = {
},
};
-int __init udp_offload_init(void)
+int udpv6_offload_init(void)
{
return inet6_add_offload(&udpv6_offload, IPPROTO_UDP);
}
+
+int udpv6_offload_exit(void)
+{
+ return inet6_del_offload(&udpv6_offload, IPPROTO_UDP);
+}
--
2.8.0.rc2
^ permalink raw reply related
* [PATCH RFC 6/9] vxlan: change vxlan to use UDP socket GRO
From: Tom Herbert @ 2016-03-23 22:36 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1458772618-845742-1-git-send-email-tom@herbertland.com>
Adapt vxlan_gro_receive, vxlan_gro_complete to take a socket argument.
Set these functions in tunnel_config. Don't set udp_offloads any more.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
drivers/net/vxlan.c | 31 +++++++++----------------------
include/net/vxlan.h | 1 -
2 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 800106a7..d27aceb 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -551,16 +551,15 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
return vh;
}
-static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,
- struct sk_buff *skb,
- struct udp_offload *uoff)
+static struct sk_buff **vxlan_gro_receive(struct sock *sk,
+ struct sk_buff **head,
+ struct sk_buff *skb)
{
struct sk_buff *p, **pp = NULL;
struct vxlanhdr *vh, *vh2;
unsigned int hlen, off_vx;
int flush = 1;
- struct vxlan_sock *vs = container_of(uoff, struct vxlan_sock,
- udp_offloads);
+ struct vxlan_sock *vs = rcu_dereference_sk_user_data(sk);
__be32 flags;
struct gro_remcsum grc;
@@ -613,8 +612,7 @@ out:
return pp;
}
-static int vxlan_gro_complete(struct sk_buff *skb, int nhoff,
- struct udp_offload *uoff)
+static int vxlan_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
{
udp_tunnel_gro_complete(skb, nhoff);
@@ -629,13 +627,6 @@ static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
struct net *net = sock_net(sk);
sa_family_t sa_family = vxlan_get_sk_family(vs);
__be16 port = inet_sk(sk)->inet_sport;
- int err;
-
- if (sa_family == AF_INET) {
- err = udp_add_offload(net, &vs->udp_offloads);
- if (err)
- pr_warn("vxlan: udp_add_offload failed with status %d\n", err);
- }
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
@@ -662,9 +653,6 @@ static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
port);
}
rcu_read_unlock();
-
- if (sa_family == AF_INET)
- udp_del_offload(&vs->udp_offloads);
}
/* Add new entry to forwarding table -- assumes lock held */
@@ -2643,21 +2631,20 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
atomic_set(&vs->refcnt, 1);
vs->flags = (flags & VXLAN_F_RCV_FLAGS);
- /* Initialize the vxlan udp offloads structure */
- vs->udp_offloads.port = port;
- vs->udp_offloads.callbacks.gro_receive = vxlan_gro_receive;
- vs->udp_offloads.callbacks.gro_complete = vxlan_gro_complete;
-
spin_lock(&vn->sock_lock);
hlist_add_head_rcu(&vs->hlist, vs_head(net, port));
vxlan_notify_add_rx_port(vs);
spin_unlock(&vn->sock_lock);
/* Mark socket as an encapsulation socket. */
+ memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
tunnel_cfg.sk_user_data = vs;
tunnel_cfg.encap_type = 1;
tunnel_cfg.encap_rcv = vxlan_rcv;
tunnel_cfg.encap_destroy = NULL;
+ tunnel_cfg.encap_fast = 0;
+ tunnel_cfg.gro_receive = vxlan_gro_receive;
+ tunnel_cfg.gro_complete = vxlan_gro_complete;
setup_udp_tunnel_sock(net, sock, &tunnel_cfg);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index a763c96..b4ba1d73 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -131,7 +131,6 @@ struct vxlan_sock {
struct rcu_head rcu;
struct hlist_head vni_list[VNI_HASH_SIZE];
atomic_t refcnt;
- struct udp_offload udp_offloads;
u32 flags;
};
--
2.8.0.rc2
^ permalink raw reply related
* [PATCH RFC 8/9] geneve: change to use UDP socket GRO
From: Tom Herbert @ 2016-03-23 22:36 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1458772618-845742-1-git-send-email-tom@herbertland.com>
Adapt geneve_gro_receive, geneve_gro_complete to take a socket argument.
Set these functions in tunnel_config. Don't set udp_offloads any more.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
drivers/net/geneve.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 192631a..6caf9ed 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -87,7 +87,6 @@ struct geneve_sock {
struct socket *sock;
struct rcu_head rcu;
int refcnt;
- struct udp_offload udp_offloads;
struct hlist_head vni_list[VNI_HASH_SIZE];
u32 flags;
};
@@ -409,14 +408,6 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
struct net *net = sock_net(sk);
sa_family_t sa_family = geneve_get_sk_family(gs);
__be16 port = inet_sk(sk)->inet_sport;
- int err;
-
- if (sa_family == AF_INET) {
- err = udp_add_offload(sock_net(sk), &gs->udp_offloads);
- if (err)
- pr_warn("geneve: udp_add_offload failed with status %d\n",
- err);
- }
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
@@ -432,9 +423,9 @@ static int geneve_hlen(struct genevehdr *gh)
return sizeof(*gh) + gh->opt_len * 4;
}
-static struct sk_buff **geneve_gro_receive(struct sk_buff **head,
- struct sk_buff *skb,
- struct udp_offload *uoff)
+static struct sk_buff **geneve_gro_receive(struct sock *sk,
+ struct sk_buff **head,
+ struct sk_buff *skb)
{
struct sk_buff *p, **pp = NULL;
struct genevehdr *gh, *gh2;
@@ -495,8 +486,8 @@ out:
return pp;
}
-static int geneve_gro_complete(struct sk_buff *skb, int nhoff,
- struct udp_offload *uoff)
+static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,
+ int nhoff)
{
struct genevehdr *gh;
struct packet_offload *ptype;
@@ -545,14 +536,15 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
INIT_HLIST_HEAD(&gs->vni_list[h]);
/* Initialize the geneve udp offloads structure */
- gs->udp_offloads.port = port;
- gs->udp_offloads.callbacks.gro_receive = geneve_gro_receive;
- gs->udp_offloads.callbacks.gro_complete = geneve_gro_complete;
geneve_notify_add_rx_port(gs);
/* Mark socket as an encapsulation socket */
+ memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
tunnel_cfg.sk_user_data = gs;
tunnel_cfg.encap_type = 1;
+ tunnel_cfg.encap_fast = 0;
+ tunnel_cfg.gro_receive = geneve_gro_receive;
+ tunnel_cfg.gro_complete = geneve_gro_complete;
tunnel_cfg.encap_rcv = geneve_udp_encap_recv;
tunnel_cfg.encap_destroy = NULL;
setup_udp_tunnel_sock(net, sock, &tunnel_cfg);
@@ -576,9 +568,6 @@ static void geneve_notify_del_rx_port(struct geneve_sock *gs)
}
rcu_read_unlock();
-
- if (sa_family == AF_INET)
- udp_del_offload(&gs->udp_offloads);
}
static void __geneve_sock_release(struct geneve_sock *gs)
--
2.8.0.rc2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox