* Re: [3.2.y] e1000: fix vlan processing regression
From: Ben Hutchings @ 2012-10-27 23:11 UTC (permalink / raw)
To: Jonathan Nieder
Cc: David Miller, Jeff Kirsher, Jiri Pirko, netdev, Andy Gospodarek,
sassmann, Andrey Jr. Melnikov, stable
In-Reply-To: <20121024012118.GA6898@elie.Belkin>
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
On Tue, 2012-10-23 at 18:21 -0700, Jonathan Nieder wrote:
> From: Jiri Pirko <jpirko@redhat.com>
> Date: Tue, 20 Mar 2012 18:10:01 +0000
>
> commit 52f5509fe8ccb607ff9b84ad618f244262336475 upstream.
>
> This patch fixes a regression introduced by commit "e1000: do vlan
> cleanup (799d531)".
>
> Apparently some e1000 chips (not mine) are sensitive about the order of
> setting vlan filter and vlan stripping/inserting functionality. So this
> patch changes the order so it's the same as before vlan cleanup.
[...]
Added to the queue, thanks.
Ben.
--
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: Information about napi_struct and net_device
From: Eric Dumazet @ 2012-10-27 23:02 UTC (permalink / raw)
To: Francois Romieu; +Cc: Javier Domingo, netdev
In-Reply-To: <20121027182207.GA12858@electric-eye.fr.zoreil.com>
On Sat, 2012-10-27 at 20:22 +0200, Francois Romieu wrote:
> Not all drivers use napi_gro_receive/napi_gro_flush.
>
Yes, note that I only mentioned "grep napi", not these specific verbs
Also, napi_gro_receive() could be used now GRO doesnt mandate hardware
checksums.
> It could be worth comparing with drivers/net/ethernet/via/via-rhine.c (small)
> or drivers/net/ethernet/broadcom/tg3.c (bigger).
Unfortunately tg3.c uses internal stuff sith napi substring, so I
ignored it :
$ grep napi drivers/net/ethernet/broadcom/tg3.c | wc -l
376
^ permalink raw reply
* Re: [PATCH net-next] tcp: better retrans tracking for defer-accept
From: Julian Anastasov @ 2012-10-27 22:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Vijay Subramanian, netdev, ncardwell,
Venkat Venkatsubra, Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351347537.30380.315.camel@edumazet-glaptop>
Hello,
On Sat, 27 Oct 2012, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> For passive TCP connections using TCP_DEFER_ACCEPT facility,
> we incorrectly increment req->retrans each time timeout triggers
> while no SYNACK is sent.
>
> SYNACK are not sent for TCP_DEFER_ACCEPT that were established (for wich
> we received the ACK from client). Only the last SYNACK is
> sent so that we can receive again an ACK from client, to move the
> req into accept queue. We plan to change this later to avoid
> the useless retransmit (and potential problem as this SYNACK could be
> lost)
>
> TCP_INFO later gives wrong information to user, claiming imaginary
> retransmits.
>
> Decouple req->retrans field into two independent fields :
>
> num_retrans : number of retransmit
> num_timeout : number of timeouts
>
> num_timeout is the counter that is incremented at each timeout,
> regardless of actual SYNACK being sent or not, and used to
> compute the exponential timeout.
>
> Introduce inet_rtx_syn_ack() helper to increment num_retrans
> only if ->rtx_syn_ack() succeeded.
>
> Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
> when we re-send a SYNACK in answer to a (retransmitted) SYN.
> Prior to this patch, we were not counting these retransmits.
>
> Change tcp_v[46]_rtx_synack() to increment TCP_MIB_RETRANSSEGS
> only if a synack packet was successfully queued.
>
> Reported-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Vijay Subramanian <subramanian.vijay@gmail.com>
> Cc: Elliott Hughes <enh@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index ba48e79..b236ef0 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -340,7 +340,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
> }
>
> req->expires = 0UL;
> - req->retrans = 0;
> + req->num_retrans = 0;
also req->num_timeout = 0; ?
>
> /*
> * We need to lookup the route here to get at the correct
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index 182ab9a..4016197 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -214,7 +214,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
> ireq6->iif = inet6_iif(skb);
>
> req->expires = 0UL;
> - req->retrans = 0;
> + req->num_retrans = 0;
also req->num_timeout = 0; ?
> ireq->ecn_ok = ecn_ok;
> ireq->snd_wscale = tcp_opt.snd_wscale;
> ireq->sack_ok = tcp_opt.sack_ok;
> @@ -1866,7 +1869,7 @@ static void get_openreq6(struct seq_file *seq,
> 0,0, /* could print option size, but that is af dependent. */
> 1, /* timers active (only the expire timer) */
> jiffies_to_clock_t(ttd),
> - req->retrans,
> + req->num_timeout,
num_timeout already noted by Neal
> from_kuid_munged(seq_user_ns(seq), uid),
> 0, /* non standard timer */
> 0, /* open_requests have no inode */
I don't see any problems with such patch, just
check if it is for correct tree, I see some atomic operations
with qlen_young in patch.
One thing to finally decide: should we use limit for
retransmissions or for timeout, is the following better?:
if (!rskq_defer_accept) {
*expire = req->num_retrans >= thresh;
^^^^^^^^^^^
*resend = 1;
return;
}
*expire = req->num_retrans >= thresh &&
^^^^^^^^^^^
(!inet_rsk(req)->acked || req->num_timeout >= max_retries);
Same in tcp_fastopen_synack_timer:
if (req->num_retrans >= max_retries) {
^^^^^^^^^^^
By this way limit (thresh) is for retransmissions while
defer period should be time based as before.
Documentation/networking/ip-sysctl.txt compares
the tcp_synack_retries with timeout, so I'm not sure
which variant is better to use. OTOH, include/net/tcp.h
does not specify minimum in seconds for TCP_SYNACK_RETRIES,
so what should we prefer, the count or its time form?
May be count (num_retrans) is better against SYN attacks.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: Information about napi_struct and net_device
From: Rami Rosen @ 2012-10-27 20:10 UTC (permalink / raw)
To: Javier Domingo; +Cc: netdev
In-Reply-To: <CALZVap=ybdWdK22LHsfSRu+TVCJnvL95KeChtCdX8DE-C=1gAg@mail.gmail.com>
Hi,
You can find some info about net_device in this wiki page:
https://www.linuxfoundation.org/collaborate/workgroups/networking/networkoverview
A shameless plug - I wrote it.
Regards,
Rami Rosen
http://ramirose.wix.com/ramirosen
On Sat, Oct 27, 2012 at 6:18 PM, Javier Domingo <javierdo1@gmail.com> wrote:
> Hello,
>
> I am updating a kernel patch from 2.6.23-rc7 to v3.6, using git
> facilities, and I have found that there have been lots of changes in
> networking. Now the drivers are all by vendor, and more important,
> net_device is not used always, as napi_struct took its place.
>
> I have asked in #kernel but no one answered me, so I subscribed here.
> Is there any place I can read about how is it now designed the use of
> napi_struct and net_device? I have found a presentation[1] about how
> networking was changing etc, and I wondered if there is anything more
> precise about how it is actually implemented.
>
> I understand a little why napi_struct, but don't really know much
> about the change,
>
> Hope someone can help me,
>
> Javier Domingo
>
>
> [1]: http://vger.kernel.org/~davem/davem_nyc09.pdf
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Information about napi_struct and net_device
From: Francois Romieu @ 2012-10-27 18:22 UTC (permalink / raw)
To: Javier Domingo; +Cc: Eric Dumazet, netdev
In-Reply-To: <1351357421.30380.328.camel@edumazet-glaptop>
Eric Dumazet <eric.dumazet@gmail.com> :
[...]
> # grep -n napi drivers/net/ethernet/realtek/8139cp.c
> 327: struct napi_struct napi;
> 436: napi_gro_receive(&cp->napi, skb);
> 468:static int cp_rx_poll(struct napi_struct *napi, int budget)
> 470: struct cp_private *cp = container_of(napi, struct cp_private, napi);
> 566: napi_gro_flush(napi, false);
> 568: __napi_complete(napi);
> 605: if (napi_schedule_prep(&cp->napi)) {
> 607: __napi_schedule(&cp->napi);
> 1132: napi_enable(&cp->napi);
> 1149: napi_disable(&cp->napi);
> 1160: napi_disable(&cp->napi);
> 1933: netif_napi_add(dev, &cp->napi, cp_rx_poll, 16);
Not all drivers use napi_gro_receive/napi_gro_flush.
It could be worth comparing with drivers/net/ethernet/via/via-rhine.c (small)
or drivers/net/ethernet/broadcom/tg3.c (bigger).
--
Ueimor
^ permalink raw reply
* Re: [net-next 02/12] ixgbe: Fix return value from macvlan filter function
From: Konstantin Stepanyuk @ 2012-10-27 18:36 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Greg Rose, netdev, gospo, sassmann
In-Reply-To: <1351296518-31174-3-git-send-email-jeffrey.t.kirsher@intel.com>
On Fri, Oct 26, 2012 at 6:08 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> The function to set the macvlan filter should return success or failure
> instead of the index of the filter. The message processing function was
> misinterpreting the index as a non-zero return code indicating failure and
> NACKing MAC filter set messages that actually succeeded.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Tested-by: Robert Garrett <robertx.e.garrett@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 96876b7..b68bf0f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -742,7 +742,8 @@ static int ixgbe_set_vf_macvlan_msg(struct ixgbe_adapter *adapter,
> e_warn(drv,
> "VF %d has requested a MACVLAN filter but there is no space for it\n",
> vf);
> - return err;
> +
> + return !!err < 0;
This expression always resolves to false, either 0 < 0 or 1 < 0,
because '!' has higher precedence than '<'.
Thanks,
Konstantin
^ permalink raw reply
* Re: [PATCH net-next] tcp: better retrans tracking for defer-accept
From: Neal Cardwell @ 2012-10-27 18:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: Julian Anastasov, David Miller, Vijay Subramanian, Netdev,
Venkat Venkatsubra, Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351347537.30380.315.camel@edumazet-glaptop>
On Sat, Oct 27, 2012 at 10:18 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
> @@ -2638,7 +2642,7 @@ static void get_openreq4(const struct sock *sk, const struct request_sock *req,
...
> - req->retrans,
> + req->num_retrans,
...
> @@ -1866,7 +1869,7 @@ static void get_openreq6(struct seq_file *seq,
...
> - req->retrans,
> + req->num_timeout,
...
There's a slight inconsistency in the changes to get_openreq4() and
get_openreq6() - the first now uses num_retrans and the second now
uses num_timeout.
Otherwise LGTM.
neal
^ permalink raw reply
* Re: Information about napi_struct and net_device
From: Eric Dumazet @ 2012-10-27 17:03 UTC (permalink / raw)
To: Javier Domingo; +Cc: netdev
In-Reply-To: <CALZVap=ybdWdK22LHsfSRu+TVCJnvL95KeChtCdX8DE-C=1gAg@mail.gmail.com>
On Sat, 2012-10-27 at 18:18 +0200, Javier Domingo wrote:
> Hello,
>
> I am updating a kernel patch from 2.6.23-rc7 to v3.6, using git
> facilities, and I have found that there have been lots of changes in
> networking. Now the drivers are all by vendor, and more important,
> net_device is not used always, as napi_struct took its place.
>
Yes, some directories were created and files moved around.
Not really a big deal (no real code change)
> I have asked in #kernel but no one answered me, so I subscribed here.
> Is there any place I can read about how is it now designed the use of
> napi_struct and net_device? I have found a presentation[1] about how
> networking was changing etc, and I wondered if there is anything more
> precise about how it is actually implemented.
>
> I understand a little why napi_struct, but don't really know much
> about the change,
>
> Hope someone can help me,
Seems quite straightforward, we probably should add some docs in
Documentation/networking/netdevices.txt
You could take a look at a small driver like
drivers/net/ethernet/realtek/8139cp.c
and grep for "napi" in it
# grep -n napi drivers/net/ethernet/realtek/8139cp.c
327: struct napi_struct napi;
436: napi_gro_receive(&cp->napi, skb);
468:static int cp_rx_poll(struct napi_struct *napi, int budget)
470: struct cp_private *cp = container_of(napi, struct cp_private, napi);
566: napi_gro_flush(napi, false);
568: __napi_complete(napi);
605: if (napi_schedule_prep(&cp->napi)) {
607: __napi_schedule(&cp->napi);
1132: napi_enable(&cp->napi);
1149: napi_disable(&cp->napi);
1160: napi_disable(&cp->napi);
1933: netif_napi_add(dev, &cp->napi, cp_rx_poll, 16);
^ permalink raw reply
* Information about napi_struct and net_device
From: Javier Domingo @ 2012-10-27 16:18 UTC (permalink / raw)
To: netdev
Hello,
I am updating a kernel patch from 2.6.23-rc7 to v3.6, using git
facilities, and I have found that there have been lots of changes in
networking. Now the drivers are all by vendor, and more important,
net_device is not used always, as napi_struct took its place.
I have asked in #kernel but no one answered me, so I subscribed here.
Is there any place I can read about how is it now designed the use of
napi_struct and net_device? I have found a presentation[1] about how
networking was changing etc, and I wondered if there is anything more
precise about how it is actually implemented.
I understand a little why napi_struct, but don't really know much
about the change,
Hope someone can help me,
Javier Domingo
[1]: http://vger.kernel.org/~davem/davem_nyc09.pdf
^ permalink raw reply
* Re: [PATCH net-next] sctp: support per-association stats via a new SCTP_GET_ASSOC_STATS call
From: Vlad Yasevich @ 2012-10-27 15:50 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Michele Baldessari, Neil Horman, linux-sctp@vger.kernel.org,
David S. Miller, netdev@vger.kernel.org, Thomas Graf
In-Reply-To: <DB10D819-2751-4467-858A-C4ED1DBED9F4@gmail.com>
On Oct 27, 2012, at 11:48 AM, Vlad Yasevich <vyasevich@gmail.com> wrote:
>
>
>
>
> On Oct 27, 2012, at 7:35 AM, Michele Baldessari <michele@acksyn.org> wrote:
>
>> Hi Neil & Vlad,
>>
>> On Fri, Oct 26, 2012 at 10:37:04AM -0400, Neil Horman wrote:
>>> We already have files in /proc/net/sctp to count snmp system-wide totals,
>>> per-endpoint totals, and per association totals. Why do these stats differently
>>> instead of just adding them the per-association file? I get that solaris does
>>> this, but its not codified in any of the RFC's or other standards. I would
>>> really rather see something like this go into the interfaces we have, rather
>>> than creating a new one.
>>>
>>> I also am a bit confused regarding the stats themselves. Most are fairly clear,
>>> but some seem lacking (you count most things sent and received, but only count
>>> received gap acks). Others seems vague and or confusing (when counting
>>> retransmitted chunks and packets, how do you count a packet that has both new
>>> and retransmitted chunks)? And the max observed rto stat is just odd. Each
>>> transport has an rto value, not each association, and you cal already see the
>>> individual transport rto values in /proc/net/sctp/remaddr.
>>
>> thanks a lot for your time reviewing this. I will try to address all
>> your comments in a second version of the patch. One thing I am not too
>> sure though: do you prefer me extending /proc/net/sctp/* or implement a
>> new call.
>>
>> I ask because from a previous private communication with Vlad the new
>> socket option seemed to be the preferred approach.
>> I am fine either way just let me know ;)
>
>
> socket option is preferable as /proc doesn't scale very well as number of associations grows.
One thing of note that I didn't think of before is that you want his option to be in the lksctp private space, not the API spec space.
>
> -Vlad
>
>>
>> cheers,
>> --
>> Michele Baldessari <michele@acksyn.org>
>> C2A5 9DA3 9961 4FFB E01B D0BC DDD4 DCCB 7515 5C6D
^ permalink raw reply
* [PATCH 1/1] net: macb: add pinctrl consumer support
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-27 15:49 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, netdev
If no pinctrl available just report a warning as some architecture may not
need to do anything.
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
drivers/net/ethernet/cadence/macb.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 033064b..e5fdf8a 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -26,6 +26,9 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_net.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/pinctrl/consumer.h>
#include "macb.h"
@@ -1306,6 +1309,7 @@ static int __init macb_probe(struct platform_device *pdev)
struct phy_device *phydev;
u32 config;
int err = -ENXIO;
+ struct pinctrl *pinctrl;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
@@ -1313,6 +1317,15 @@ static int __init macb_probe(struct platform_device *pdev)
goto err_out;
}
+ pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+ if (IS_ERR(pinctrl)) {
+ err = PTR_ERR(pinctrl);
+ if (err == -EPROBE_DEFER)
+ return err;
+
+ dev_warn(&pdev->dev, "No pinctrl provided\n");
+ }
+
err = -ENOMEM;
dev = alloc_etherdev(sizeof(*bp));
if (!dev)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net-next] sctp: support per-association stats via a new SCTP_GET_ASSOC_STATS call
From: Vlad Yasevich @ 2012-10-27 15:48 UTC (permalink / raw)
To: Michele Baldessari
Cc: Neil Horman, linux-sctp@vger.kernel.org, David S. Miller,
netdev@vger.kernel.org, Thomas Graf
In-Reply-To: <20121027113541.GA7966@marquez.int.rhx>
On Oct 27, 2012, at 7:35 AM, Michele Baldessari <michele@acksyn.org> wrote:
> Hi Neil & Vlad,
>
> On Fri, Oct 26, 2012 at 10:37:04AM -0400, Neil Horman wrote:
>> We already have files in /proc/net/sctp to count snmp system-wide totals,
>> per-endpoint totals, and per association totals. Why do these stats differently
>> instead of just adding them the per-association file? I get that solaris does
>> this, but its not codified in any of the RFC's or other standards. I would
>> really rather see something like this go into the interfaces we have, rather
>> than creating a new one.
>>
>> I also am a bit confused regarding the stats themselves. Most are fairly clear,
>> but some seem lacking (you count most things sent and received, but only count
>> received gap acks). Others seems vague and or confusing (when counting
>> retransmitted chunks and packets, how do you count a packet that has both new
>> and retransmitted chunks)? And the max observed rto stat is just odd. Each
>> transport has an rto value, not each association, and you cal already see the
>> individual transport rto values in /proc/net/sctp/remaddr.
>
> thanks a lot for your time reviewing this. I will try to address all
> your comments in a second version of the patch. One thing I am not too
> sure though: do you prefer me extending /proc/net/sctp/* or implement a
> new call.
>
> I ask because from a previous private communication with Vlad the new
> socket option seemed to be the preferred approach.
> I am fine either way just let me know ;)
socket option is preferable as /proc doesn't scale very well as number of associations grows.
-Vlad
>
> cheers,
> --
> Michele Baldessari <michele@acksyn.org>
> C2A5 9DA3 9961 4FFB E01B D0BC DDD4 DCCB 7515 5C6D
^ permalink raw reply
* [PATCH net-next] tcp: better retrans tracking for defer-accept
From: Eric Dumazet @ 2012-10-27 14:18 UTC (permalink / raw)
To: Julian Anastasov, David Miller
Cc: Vijay Subramanian, netdev, ncardwell, Venkat Venkatsubra,
Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351344725.30380.286.camel@edumazet-glaptop>
From: Eric Dumazet <edumazet@google.com>
For passive TCP connections using TCP_DEFER_ACCEPT facility,
we incorrectly increment req->retrans each time timeout triggers
while no SYNACK is sent.
SYNACK are not sent for TCP_DEFER_ACCEPT that were established (for wich
we received the ACK from client). Only the last SYNACK is
sent so that we can receive again an ACK from client, to move the
req into accept queue. We plan to change this later to avoid
the useless retransmit (and potential problem as this SYNACK could be
lost)
TCP_INFO later gives wrong information to user, claiming imaginary
retransmits.
Decouple req->retrans field into two independent fields :
num_retrans : number of retransmit
num_timeout : number of timeouts
num_timeout is the counter that is incremented at each timeout,
regardless of actual SYNACK being sent or not, and used to
compute the exponential timeout.
Introduce inet_rtx_syn_ack() helper to increment num_retrans
only if ->rtx_syn_ack() succeeded.
Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
when we re-send a SYNACK in answer to a (retransmitted) SYN.
Prior to this patch, we were not counting these retransmits.
Change tcp_v[46]_rtx_synack() to increment TCP_MIB_RETRANSSEGS
only if a synack packet was successfully queued.
Reported-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Vijay Subramanian <subramanian.vijay@gmail.com>
Cc: Elliott Hughes <enh@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
include/net/request_sock.h | 12 ++++++++----
net/dccp/minisocks.c | 3 +--
net/ipv4/inet_connection_sock.c | 25 ++++++++++++++++++-------
net/ipv4/inet_diag.c | 2 +-
net/ipv4/syncookies.c | 2 +-
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_ipv4.c | 16 ++++++++++------
net/ipv4/tcp_minisocks.c | 8 ++++----
net/ipv4/tcp_timer.c | 8 ++++----
net/ipv6/syncookies.c | 2 +-
net/ipv6/tcp_ipv6.c | 11 +++++++----
11 files changed, 56 insertions(+), 35 deletions(-)
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 5a7b6c2..3ed0897 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -49,13 +49,16 @@ struct request_sock_ops {
struct request_sock *req);
};
+extern int inet_rtx_syn_ack(struct sock *parent, struct request_sock *req);
+
/* struct request_sock - mini sock to represent a connection request
*/
struct request_sock {
struct request_sock *dl_next; /* Must be first member! */
u16 mss;
- u8 retrans;
- u8 cookie_ts; /* syncookie: encode tcpopts in timestamp */
+ u8 num_retrans; /* number of retransmits */
+ u8 cookie_ts:1; /* syncookie: encode tcpopts in timestamp */
+ u8 num_timeout:7; /* number of timeouts */
/* The following two fields can be easily recomputed I think -AK */
u32 window_clamp; /* window clamp at creation time */
u32 rcv_wnd; /* rcv_wnd offered first time */
@@ -233,7 +236,7 @@ static inline int reqsk_queue_removed(struct request_sock_queue *queue,
{
struct listen_sock *lopt = queue->listen_opt;
- if (req->retrans == 0)
+ if (req->num_timeout == 0)
atomic_dec(&lopt->qlen_young);
return atomic_dec_return(&lopt->qlen);
@@ -271,7 +274,8 @@ static inline void reqsk_queue_hash_req(struct request_sock_queue *queue,
struct listen_sock *lopt = queue->listen_opt;
req->expires = jiffies + timeout;
- req->retrans = 0;
+ req->num_retrans = 0;
+ req->num_timeout = 0;
req->sk = NULL;
req->dl_next = lopt->syn_table[hash];
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index ea850ce..662071b 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -174,8 +174,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
* To protect against Request floods, increment retrans
* counter (backoff, monitored by dccp_response_timer).
*/
- req->retrans++;
- req->rsk_ops->rtx_syn_ack(sk, req, NULL);
+ inet_rtx_syn_ack(sk, req);
}
/* Network Duplicate, discard packet */
return NULL;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index bee9bd4..4727538 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -521,21 +521,31 @@ static inline void syn_ack_recalc(struct request_sock *req, const int thresh,
int *expire, int *resend)
{
if (!rskq_defer_accept) {
- *expire = req->retrans >= thresh;
+ *expire = req->num_timeout >= thresh;
*resend = 1;
return;
}
- *expire = req->retrans >= thresh &&
- (!inet_rsk(req)->acked || req->retrans >= max_retries);
+ *expire = req->num_timeout >= thresh &&
+ (!inet_rsk(req)->acked || req->num_timeout >= max_retries);
/*
* Do not resend while waiting for data after ACK,
* start to resend on end of deferring period to give
* last chance for data or ACK to create established socket.
*/
*resend = !inet_rsk(req)->acked ||
- req->retrans >= rskq_defer_accept - 1;
+ req->num_timeout >= rskq_defer_accept - 1;
}
+int inet_rtx_syn_ack(struct sock *parent, struct request_sock *req)
+{
+ int err = req->rsk_ops->rtx_syn_ack(parent, req, NULL);
+
+ if (!err)
+ req->num_retrans++;
+ return err;
+}
+EXPORT_SYMBOL(inet_rtx_syn_ack);
+
void inet_csk_reqsk_queue_prune(struct sock *parent,
const unsigned long interval,
const unsigned long timeout,
@@ -599,13 +609,14 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
req->rsk_ops->syn_ack_timeout(parent, req);
if (!expire &&
(!resend ||
- !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
+ !inet_rtx_syn_ack(parent, req) ||
inet_rsk(req)->acked)) {
unsigned long timeo;
- if (req->retrans++ == 0)
+ if (req->num_timeout++ == 0)
atomic_dec(&lopt->qlen_young);
- timeo = min((timeout << req->retrans), max_rto);
+ timeo = min(timeout << req->num_timeout,
+ max_rto);
req->expires = now + timeo;
reqp = &req->dl_next;
continue;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index b29caa7..e511a35 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -620,7 +620,7 @@ static int inet_diag_fill_req(struct sk_buff *skb, struct sock *sk,
r->idiag_family = sk->sk_family;
r->idiag_state = TCP_SYN_RECV;
r->idiag_timer = 1;
- r->idiag_retrans = req->retrans;
+ r->idiag_retrans = req->num_retrans;
r->id.idiag_if = sk->sk_bound_dev_if;
sock_diag_save_cookie(req, r->id.idiag_cookie);
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index ba48e79..b236ef0 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -340,7 +340,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
}
req->expires = 0UL;
- req->retrans = 0;
+ req->num_retrans = 0;
/*
* We need to lookup the route here to get at the correct
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 60cf836..e95b4e5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5991,7 +5991,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
*/
if (req) {
tcp_synack_rtt_meas(sk, req);
- tp->total_retrans = req->retrans;
+ tp->total_retrans = req->num_retrans;
reqsk_fastopen_remove(sk, req, false);
} else {
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 694ea4c..20849cc 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -877,10 +877,13 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
}
static int tcp_v4_rtx_synack(struct sock *sk, struct request_sock *req,
- struct request_values *rvp)
+ struct request_values *rvp)
{
- TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_RETRANSSEGS);
- return tcp_v4_send_synack(sk, NULL, req, rvp, 0, false);
+ int res = tcp_v4_send_synack(sk, NULL, req, rvp, 0, false);
+
+ if (!res)
+ TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_RETRANSSEGS);
+ return res;
}
/*
@@ -1386,7 +1389,8 @@ static int tcp_v4_conn_req_fastopen(struct sock *sk,
struct sock *child;
int err;
- req->retrans = 0;
+ req->num_retrans = 0;
+ req->num_timeout = 0;
req->sk = NULL;
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL);
@@ -1740,7 +1744,7 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
tcp_initialize_rcv_mss(newsk);
tcp_synack_rtt_meas(newsk, req);
- newtp->total_retrans = req->retrans;
+ newtp->total_retrans = req->num_retrans;
#ifdef CONFIG_TCP_MD5SIG
/* Copy over the MD5 key from the original socket */
@@ -2638,7 +2642,7 @@ static void get_openreq4(const struct sock *sk, const struct request_sock *req,
0, 0, /* could print option size, but that is af dependent. */
1, /* timers active (only the expire timer) */
jiffies_delta_to_clock_t(delta),
- req->retrans,
+ req->num_retrans,
from_kuid_munged(seq_user_ns(f), uid),
0, /* non standard timer */
0, /* open_requests have no inode */
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 27536ba..0404b3f 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -552,7 +552,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* it can be estimated (approximately)
* from another data.
*/
- tmp_opt.ts_recent_stamp = get_seconds() - ((TCP_TIMEOUT_INIT/HZ)<<req->retrans);
+ tmp_opt.ts_recent_stamp = get_seconds() - ((TCP_TIMEOUT_INIT/HZ)<<req->num_timeout);
paws_reject = tcp_paws_reject(&tmp_opt, th->rst);
}
}
@@ -581,7 +581,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* Note that even if there is new data in the SYN packet
* they will be thrown away too.
*/
- req->rsk_ops->rtx_syn_ack(sk, req, NULL);
+ inet_rtx_syn_ack(sk, req);
return NULL;
}
@@ -695,7 +695,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
/* Got ACK for our SYNACK, so update baseline for SYNACK RTT sample. */
if (tmp_opt.saw_tstamp && tmp_opt.rcv_tsecr)
tcp_rsk(req)->snt_synack = tmp_opt.rcv_tsecr;
- else if (req->retrans) /* don't take RTT sample if retrans && ~TS */
+ else if (req->num_retrans) /* don't take RTT sample if retrans && ~TS */
tcp_rsk(req)->snt_synack = 0;
/* For Fast Open no more processing is needed (sk is the
@@ -705,7 +705,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
return sk;
/* While TCP_DEFER_ACCEPT is active, drop bare ACK. */
- if (req->retrans < inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
+ if (req->num_timeout < inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
inet_rsk(req)->acked = 1;
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPDEFERACCEPTDROP);
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index fc04711..62c69ab 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -318,7 +318,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk)
req = tcp_sk(sk)->fastopen_rsk;
req->rsk_ops->syn_ack_timeout(sk, req);
- if (req->retrans >= max_retries) {
+ if (req->num_timeout >= max_retries) {
tcp_write_err(sk);
return;
}
@@ -327,10 +327,10 @@ static void tcp_fastopen_synack_timer(struct sock *sk)
* regular retransmit because if the child socket has been accepted
* it's not good to give up too easily.
*/
- req->rsk_ops->rtx_syn_ack(sk, req, NULL);
- req->retrans++;
+ inet_rtx_syn_ack(sk, req);
+ req->num_timeout++;
inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
- TCP_TIMEOUT_INIT << req->retrans, TCP_RTO_MAX);
+ TCP_TIMEOUT_INIT << req->num_timeout, TCP_RTO_MAX);
}
/*
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index 182ab9a..4016197 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -214,7 +214,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
ireq6->iif = inet6_iif(skb);
req->expires = 0UL;
- req->retrans = 0;
+ req->num_retrans = 0;
ireq->ecn_ok = ecn_ok;
ireq->snd_wscale = tcp_opt.snd_wscale;
ireq->sack_ok = tcp_opt.sack_ok;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index bb6782e..c73d0eb 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -495,9 +495,12 @@ static int tcp_v6_rtx_synack(struct sock *sk, struct request_sock *req,
struct request_values *rvp)
{
struct flowi6 fl6;
+ int res;
- TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_RETRANSSEGS);
- return tcp_v6_send_synack(sk, NULL, &fl6, req, rvp, 0);
+ res = tcp_v6_send_synack(sk, NULL, &fl6, req, rvp, 0);
+ if (!res)
+ TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_RETRANSSEGS);
+ return res;
}
static void tcp_v6_reqsk_destructor(struct request_sock *req)
@@ -1364,7 +1367,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
tcp_initialize_rcv_mss(newsk);
tcp_synack_rtt_meas(newsk, req);
- newtp->total_retrans = req->retrans;
+ newtp->total_retrans = req->num_retrans;
newinet->inet_daddr = newinet->inet_saddr = LOOPBACK4_IPV6;
newinet->inet_rcv_saddr = LOOPBACK4_IPV6;
@@ -1866,7 +1869,7 @@ static void get_openreq6(struct seq_file *seq,
0,0, /* could print option size, but that is af dependent. */
1, /* timers active (only the expire timer) */
jiffies_to_clock_t(ttd),
- req->retrans,
+ req->num_timeout,
from_kuid_munged(seq_user_ns(seq), uid),
0, /* non standard timer */
0, /* open_requests have no inode */
^ permalink raw reply related
* Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Eric Dumazet @ 2012-10-27 13:32 UTC (permalink / raw)
To: Julian Anastasov
Cc: Vijay Subramanian, netdev, davem, edumazet, ncardwell,
Venkat Venkatsubra, Elliott Hughes, Yuchung Cheng
In-Reply-To: <alpine.LFD.2.00.1210271556250.1628@ja.ssi.bg>
On Sat, 2012-10-27 at 16:23 +0300, Julian Anastasov wrote:
> Hello,
>
> On Sat, 27 Oct 2012, Eric Dumazet wrote:
>
> >
> > Author: Eric Dumazet <edumazet@google.com>
> > Date: Tue Oct 2 02:21:12 2012 -0700
> >
> > net-tcp: better retrans tracking for defer-accept
> >
> > For passive TCP connections using TCP_DEFER_ACCEPT facility,
> > we incorrectly increment req->retrans each time timeout triggers
> > while no SYNACK is sent.
> >
> > Decouple req->retrans field into two fields :
> >
> > num_retrans : number of retransmit
> > num_timeout : number of timeouts
> >
> > (retrans was renamed to make sure we didnt miss an occurrence)
> >
> > introduce inet_rtx_syn_ack() helper to increment num_retrans
> > only if ->rtx_syn_ack() succeeded.
>
> This is dangerous, the first of the cases is route
> failure, what if we just added reject route for some attacker?
> We will get error forever. May be it is difficult to decide
> which error should change the counter. IMHO, such reliability
> is not needed, we can be short of memory too.
>
We increase num_timeout regardless of success or failure sending a
SYNACK (can be a route failure, a memory allocation failure, a full
qdisc...)
So its not 'forever'. The decision to abort a SYN_RECV is based on
num_timeouts only, not anymore on 'number of restransmits'
num_retrans is only counting number of SYNACKS that were sent.
num_retrans <= num_timeouts
(Usually its the same, unless you have errors, or DEFER_ACCEPT
mini-sockets)
> > Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
> > when we re-send a SYNACK in answer to a SYN. Prior to this patch,
> > we were not counting these retransmits.
>
> Such change looks correct. Of course, it has
> side effect on current TCP_DEFER_ACCEPT calculations but
> it is a TCP_DEFER_ACCEPT implementation problem.
Better wait to see the patch, it changes nothing yet for
TCP_DEFER_ACCEPT
It only changes accounting problems, for more precise tracking of tcp
stack behavior.
TCP_DEFER_ACCEPT sockets have this strange accounting bug saying that
some packets were retransmitted, while its not true : We only were
waiting the user request.
^ permalink raw reply
* Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Julian Anastasov @ 2012-10-27 13:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: Vijay Subramanian, netdev, davem, edumazet, ncardwell,
Venkat Venkatsubra, Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351339032.30380.222.camel@edumazet-glaptop>
Hello,
On Sat, 27 Oct 2012, Eric Dumazet wrote:
> > @@ -598,9 +598,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
> > &expire, &resend);
> > req->rsk_ops->syn_ack_timeout(parent, req);
> > if (!expire &&
> > - (!resend ||
> > - !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
> > - inet_rsk(req)->acked)) {
> > + (!resend || inet_rsk(req)->acked ||
> > + !req->rsk_ops->rtx_syn_ack(parent, req, NULL))) {
> > unsigned long timeo;
> >
> > if (req->retrans++ == 0)
>
>
> Part of the complexity of this is that req->retrans is the number of
> timeouts, serving as the exponential backoff base.
>
> Unfortunately we have a side effect because number of retransmits is
> wrong for defer accept.
>
> Here is what I suggest : upstream to net-next this patch we use at
> Google :
>
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Oct 2 02:21:12 2012 -0700
>
> net-tcp: better retrans tracking for defer-accept
>
> For passive TCP connections using TCP_DEFER_ACCEPT facility,
> we incorrectly increment req->retrans each time timeout triggers
> while no SYNACK is sent.
>
> Decouple req->retrans field into two fields :
>
> num_retrans : number of retransmit
> num_timeout : number of timeouts
>
> (retrans was renamed to make sure we didnt miss an occurrence)
>
> introduce inet_rtx_syn_ack() helper to increment num_retrans
> only if ->rtx_syn_ack() succeeded.
This is dangerous, the first of the cases is route
failure, what if we just added reject route for some attacker?
We will get error forever. May be it is difficult to decide
which error should change the counter. IMHO, such reliability
is not needed, we can be short of memory too.
> Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
> when we re-send a SYNACK in answer to a SYN. Prior to this patch,
> we were not counting these retransmits.
Such change looks correct. Of course, it has
side effect on current TCP_DEFER_ACCEPT calculations but
it is a TCP_DEFER_ACCEPT implementation problem.
> Change tcp_v[46]_rtx_synack() to increment TCP_MIB_RETRANSSEGS
> only if a synack packet was successfuly queued.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* [PATCH net-next 2/2] x86: bpf_jit_comp: add vlan tag support
From: Eric Dumazet @ 2012-10-27 12:26 UTC (permalink / raw)
To: David Miller; +Cc: Ani Sinha, netdev, Daniel Borkmann
From: Eric Dumazet <edumazet@google.com>
This patch is a follow-up for patch "net: filter: add vlan tag access"
to support the new VLAN_TAG/VLAN_TAG_PRESENT accessors in BPF JIT.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Ani Sinha <ani@aristanetworks.com>
Cc: Daniel Borkmann <danborkmann@iogearbox.net>
---
arch/x86/net/bpf_jit_comp.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 520d2bd..d11a470 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -11,6 +11,7 @@
#include <asm/cacheflush.h>
#include <linux/netdevice.h>
#include <linux/filter.h>
+#include <linux/if_vlan.h>
/*
* Conventions :
@@ -212,6 +213,8 @@ void bpf_jit_compile(struct sk_filter *fp)
case BPF_S_ANC_MARK:
case BPF_S_ANC_RXHASH:
case BPF_S_ANC_CPU:
+ case BPF_S_ANC_VLAN_TAG:
+ case BPF_S_ANC_VLAN_TAG_PRESENT:
case BPF_S_ANC_QUEUE:
case BPF_S_LD_W_ABS:
case BPF_S_LD_H_ABS:
@@ -515,6 +518,24 @@ void bpf_jit_compile(struct sk_filter *fp)
CLEAR_A();
#endif
break;
+ case BPF_S_ANC_VLAN_TAG:
+ case BPF_S_ANC_VLAN_TAG_PRESENT:
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, vlan_tci) != 2);
+ if (is_imm8(offsetof(struct sk_buff, vlan_tci))) {
+ /* movzwl off8(%rdi),%eax */
+ EMIT4(0x0f, 0xb7, 0x47, offsetof(struct sk_buff, vlan_tci));
+ } else {
+ EMIT3(0x0f, 0xb7, 0x87); /* movzwl off32(%rdi),%eax */
+ EMIT(offsetof(struct sk_buff, vlan_tci), 4);
+ }
+ BUILD_BUG_ON(VLAN_TAG_PRESENT != 0x1000);
+ if (filter[i].code == BPF_S_ANC_VLAN_TAG) {
+ EMIT3(0x80, 0xe4, 0xef); /* and $0xef,%ah */
+ } else {
+ EMIT3(0xc1, 0xe8, 0x0c); /* shr $0xc,%eax */
+ EMIT3(0x83, 0xe0, 0x01); /* and $0x1,%eax */
+ }
+ break;
case BPF_S_LD_W_ABS:
func = CHOOSE_LOAD_FUNC(K, sk_load_word);
common_load: seen |= SEEN_DATAREF;
^ permalink raw reply related
* [PATCH net-next 1/2] net: filter: add vlan tag access
From: Eric Dumazet @ 2012-10-27 12:26 UTC (permalink / raw)
To: David Miller; +Cc: Ani Sinha, netdev, Daniel Borkmann
From: Eric Dumazet <edumazet@google.com>
BPF filters lack ability to access skb->vlan_tci
This patch adds two new ancillary accessors :
SKF_AD_VLAN_TAG (44) mapped to vlan_tx_tag_get(skb)
SKF_AD_VLAN_TAG_PRESENT (48) mapped to vlan_tx_tag_present(skb)
This allows libpcap/tcpdump to use a kernel filter instead of
having to fallback to accept all packets, then filter them in
user space.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Ani Sinha <ani@aristanetworks.com>
Suggested-by: Daniel Borkmann <danborkmann@iogearbox.net>
---
include/linux/filter.h | 2 ++
include/uapi/linux/filter.h | 4 +++-
net/core/filter.c | 9 +++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 24d251f..c9f0005 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -123,6 +123,8 @@ enum {
BPF_S_ANC_CPU,
BPF_S_ANC_ALU_XOR_X,
BPF_S_ANC_SECCOMP_LD_W,
+ BPF_S_ANC_VLAN_TAG,
+ BPF_S_ANC_VLAN_TAG_PRESENT,
};
#endif /* __LINUX_FILTER_H__ */
diff --git a/include/uapi/linux/filter.h b/include/uapi/linux/filter.h
index 3d79224..9cfde69 100644
--- a/include/uapi/linux/filter.h
+++ b/include/uapi/linux/filter.h
@@ -127,7 +127,9 @@ struct sock_fprog { /* Required for SO_ATTACH_FILTER. */
#define SKF_AD_RXHASH 32
#define SKF_AD_CPU 36
#define SKF_AD_ALU_XOR_X 40
-#define SKF_AD_MAX 44
+#define SKF_AD_VLAN_TAG 44
+#define SKF_AD_VLAN_TAG_PRESENT 48
+#define SKF_AD_MAX 52
#define SKF_NET_OFF (-0x100000)
#define SKF_LL_OFF (-0x200000)
diff --git a/net/core/filter.c b/net/core/filter.c
index 3d92ebb..5a114d4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -39,6 +39,7 @@
#include <linux/reciprocal_div.h>
#include <linux/ratelimit.h>
#include <linux/seccomp.h>
+#include <linux/if_vlan.h>
/* No hurry in this branch
*
@@ -341,6 +342,12 @@ load_b:
case BPF_S_ANC_CPU:
A = raw_smp_processor_id();
continue;
+ case BPF_S_ANC_VLAN_TAG:
+ A = vlan_tx_tag_get(skb);
+ continue;
+ case BPF_S_ANC_VLAN_TAG_PRESENT:
+ A = !!vlan_tx_tag_present(skb);
+ continue;
case BPF_S_ANC_NLATTR: {
struct nlattr *nla;
@@ -600,6 +607,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen)
ANCILLARY(RXHASH);
ANCILLARY(CPU);
ANCILLARY(ALU_XOR_X);
+ ANCILLARY(VLAN_TAG);
+ ANCILLARY(VLAN_TAG_PRESENT);
}
}
ftest->code = code;
^ permalink raw reply related
* [PATCH net-next] net/cadence: depend on HAS_IOMEM
From: Joachim Eastwood @ 2012-10-27 12:10 UTC (permalink / raw)
To: nicolas.ferre, davem, fengguang.wu; +Cc: netdev, Joachim Eastwood
Fixes the following build failure on S390:
In file included from drivers/net/ethernet/cadence/at91_ether.c:35:0:
drivers/net/ethernet/cadence/macb.h: In function 'macb_is_gem':
drivers/net/ethernet/cadence/macb.h:563:2: error: implicit declaration of function '__raw_readl' [-Werror=implicit-function-declaration]
drivers/net/ethernet/cadence/at91_ether.c: In function 'update_mac_address':
drivers/net/ethernet/cadence/at91_ether.c:119:2: error: implicit declaration of function '__raw_writel' [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
drivers/net/ethernet/cadence/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 40172d1..ceb0de0 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -4,6 +4,7 @@
config NET_CADENCE
bool "Cadence devices"
+ depends on HAS_IOMEM
default y
---help---
If you have a network (Ethernet) card belonging to this class, say Y.
--
1.8.0
^ permalink raw reply related
* Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Eric Dumazet @ 2012-10-27 11:57 UTC (permalink / raw)
To: Vijay Subramanian
Cc: netdev, davem, edumazet, ncardwell, Venkat Venkatsubra,
Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351238750-13611-1-git-send-email-subramanian.vijay@gmail.com>
On Fri, 2012-10-26 at 01:05 -0700, Vijay Subramanian wrote:
> Elliott Hughes <enh@google.com> saw strange behavior when server socket was not
> calling accept(). Client was receiving SYN-ACK back even when socket on server
> side was not yet available. Eric noted server sockets kept resending SYN_ACKS
> and further investigation revealed the following problem.
>
> If server socket is slow to accept() connections, request_socks can represent
> connections for which the three-way handshake is already done. From client's
> point of view, the connection is in ESTABLISHED state but on server side, socket
> is not in accept_queue or ESTABLISHED state. When the syn-ack timer expires,
> because of the order in which tests are performed, server can retransmit the
> synack repeatedly. Following patch prevents the server from retransmitting the
> synack needlessly (and prevents client from replying with ack). This reduces
> traffic when server is slow to accept() connections.
>
> If the server socket has received the third ack during connection establishment,
> this is remembered in inet_rsk(req)->acked. The request_sock will expire in
> around 30 seconds and will be dropped if it does not move into accept_queue.
>
> With help from Eric Dumazet.
>
> Reported-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Tested-by: Neal Cardwell <ncardwell@google.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> Changes from V1: Changed Reported-by tag and commit message. Added Acked-by and
> Tested-by tags.
>
> Ignoring "WARNING: line over 80 characters" in the interest of readability.
>
> net/ipv4/inet_connection_sock.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index d34ce29..4e8e52e 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -598,9 +598,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
> &expire, &resend);
> req->rsk_ops->syn_ack_timeout(parent, req);
> if (!expire &&
> - (!resend ||
> - !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
> - inet_rsk(req)->acked)) {
> + (!resend || inet_rsk(req)->acked ||
> + !req->rsk_ops->rtx_syn_ack(parent, req, NULL))) {
> unsigned long timeo;
>
> if (req->retrans++ == 0)
Part of the complexity of this is that req->retrans is the number of
timeouts, serving as the exponential backoff base.
Unfortunately we have a side effect because number of retransmits is
wrong for defer accept.
Here is what I suggest : upstream to net-next this patch we use at
Google :
Author: Eric Dumazet <edumazet@google.com>
Date: Tue Oct 2 02:21:12 2012 -0700
net-tcp: better retrans tracking for defer-accept
For passive TCP connections using TCP_DEFER_ACCEPT facility,
we incorrectly increment req->retrans each time timeout triggers
while no SYNACK is sent.
Decouple req->retrans field into two fields :
num_retrans : number of retransmit
num_timeout : number of timeouts
(retrans was renamed to make sure we didnt miss an occurrence)
introduce inet_rtx_syn_ack() helper to increment num_retrans
only if ->rtx_syn_ack() succeeded.
Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
when we re-send a SYNACK in answer to a SYN. Prior to this patch,
we were not counting these retransmits.
Change tcp_v[46]_rtx_synack() to increment TCP_MIB_RETRANSSEGS
only if a synack packet was successfuly queued.
Reported-by: Yuchung Cheng <ycheng@google.com>
Then, we could more easily address this silly SYNACK syndrom.
What do you think ?
^ permalink raw reply
* Re: [PATCH net-next] sctp: support per-association stats via a new SCTP_GET_ASSOC_STATS call
From: Michele Baldessari @ 2012-10-27 11:35 UTC (permalink / raw)
To: Neil Horman, Vlad Yasevich
Cc: linux-sctp, David S. Miller, netdev, Thomas Graf
In-Reply-To: <20121026143704.GC25087@hmsreliant.think-freely.org>
Hi Neil & Vlad,
On Fri, Oct 26, 2012 at 10:37:04AM -0400, Neil Horman wrote:
> We already have files in /proc/net/sctp to count snmp system-wide totals,
> per-endpoint totals, and per association totals. Why do these stats differently
> instead of just adding them the per-association file? I get that solaris does
> this, but its not codified in any of the RFC's or other standards. I would
> really rather see something like this go into the interfaces we have, rather
> than creating a new one.
>
> I also am a bit confused regarding the stats themselves. Most are fairly clear,
> but some seem lacking (you count most things sent and received, but only count
> received gap acks). Others seems vague and or confusing (when counting
> retransmitted chunks and packets, how do you count a packet that has both new
> and retransmitted chunks)? And the max observed rto stat is just odd. Each
> transport has an rto value, not each association, and you cal already see the
> individual transport rto values in /proc/net/sctp/remaddr.
thanks a lot for your time reviewing this. I will try to address all
your comments in a second version of the patch. One thing I am not too
sure though: do you prefer me extending /proc/net/sctp/* or implement a
new call.
I ask because from a previous private communication with Vlad the new
socket option seemed to be the preferred approach.
I am fine either way just let me know ;)
cheers,
--
Michele Baldessari <michele@acksyn.org>
C2A5 9DA3 9961 4FFB E01B D0BC DDD4 DCCB 7515 5C6D
^ permalink raw reply
* Re: [RFC PATCH net-next] ipv6: remove rt6i_peer_genid from rt6_info and its handler
From: RongQing Li @ 2012-10-27 10:49 UTC (permalink / raw)
To: netdev
In-Reply-To: <1351256519-7704-1-git-send-email-roy.qing.li@gmail.com>
sorry for noise, please ignore it, and see version 2
-Roy
2012/10/26 <roy.qing.li@gmail.com>:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> 6431cbc25f(Create a mechanism for upward inetpeer propagation into routes)
> introduces these codes, but this mechanism is never enabled since
> rt6i_peer_genid always is zero whether it is not assigned or assigned by
> rt6_peer_genid(). After 5943634fc5 (ipv4: Maintain redirect and PMTU info
> in struct rtable again), the ipv4 related codes of this mechanism have been
> removed, I think we maybe able to remove them now.
^ permalink raw reply
* Re: [PATCH net-next] net: compute skb->rxhash if nic hash may be 3-tuple
From: Eric Dumazet @ 2012-10-27 10:05 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: davem, edumazet, netdev
In-Reply-To: <1351288328-30147-1-git-send-email-willemb@google.com>
On Fri, 2012-10-26 at 17:52 -0400, Willem de Bruijn wrote:
> Network device drivers can communicate a Toeplitz hash in skb->rxhash,
> but devices differ in their hashing capabilities. All compute a 5-tuple
> hash for TCP over IPv4, but for other connection-oriented protocols,
> they may compute only a 3-tuple. This breaks RPS load balancing, e.g.,
> for TCP over IPv6 flows. Additionally, for GRE and other tunnels,
> the kernel computes a 5-tuple hash over the inner packet if possible,
> but devices do not.
>
> This patch recomputes the rxhash in software in all cases where it
> cannot be certain that a 5-tuple was computed. Device drivers can avoid
> recomputation by setting the skb->l4_rxhash flag.
>
> Recomputing adds cycles to each packet when RPS is enabled or the
> packet arrives over a tunnel. A comparison of 200x TCP_STREAM between
> two servers running unmodified netnext with rxhash computation
> in hardware vs software (using ethtool -K eth0 rxhash [on|off]) shows
> how much time is spent in __skb_get_rxhash in this worst case:
>
> 0.03% swapper [kernel.kallsyms] [k] __skb_get_rxhash
> 0.03% swapper [kernel.kallsyms] [k] __skb_get_rxhash
> 0.05% swapper [kernel.kallsyms] [k] __skb_get_rxhash
>
> With 200x TCP_RR it increases to
>
> 0.10% netperf [kernel.kallsyms] [k] __skb_get_rxhash
> 0.10% netperf [kernel.kallsyms] [k] __skb_get_rxhash
> 0.10% netperf [kernel.kallsyms] [k] __skb_get_rxhash
>
> I considered having the patch explicitly skips recomputation when it knows
> that it will not improve the hash (TCP over IPv4), but that conditional
> complicates code without saving many cycles in practice, because it has
> to take place after flow dissector.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> include/linux/skbuff.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6a2c34e..a2a0bdb 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -643,7 +643,7 @@ extern unsigned int skb_find_text(struct sk_buff *skb, unsigned int from,
> extern void __skb_get_rxhash(struct sk_buff *skb);
> static inline __u32 skb_get_rxhash(struct sk_buff *skb)
> {
> - if (!skb->rxhash)
> + if (!skb->l4_rxhash)
> __skb_get_rxhash(skb);
>
> return skb->rxhash;
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Eric Dumazet @ 2012-10-27 8:50 UTC (permalink / raw)
To: Vijay Subramanian
Cc: Julian Anastasov, netdev, davem, edumazet, ncardwell,
Venkat Venkatsubra, Elliott Hughes
In-Reply-To: <CAGK4HS922-xSd0YQdE2kExzMci8A36v_m_GPa17FM7wMJ08n3w@mail.gmail.com>
On Fri, 2012-10-26 at 17:07 -0700, Vijay Subramanian wrote:
>
> Thanks for the feedback. I will wait for suggestions as to what to do
> with the patch and have another look at the code.
It seems to me current code for defer accept should be fixed as well.
'Allowing' to send the last SYNACK hoping this wont be lost somewhere
seems really strange.
syn_ack_recalc(...)
{
...
/*
* Do not resend while waiting for data after ACK,
* start to resend on end of deferring period to give
* last chance for data or ACK to create established socket.
*/
*resend = !inet_rsk(req)->acked ||
req->num_timeout >= rskq_defer_accept - 1;
}
DEFER_ACCEPT should really mean :
We have the socket, only defer putting it in the accept queue until :
- Client sent data.
Or
- A given amount of time has elapsed
Defer accept is typically used by protocols where client sends the first
chunk of data.
If after XX seconds chunk of data is still not there, sending a SYNACK
wont really help : just pass the fd to application and application
probably will wait another XX seconds for the client request, then
timeout and close the socket.
So instead of sending a final SYNACK just to be able to receive an ACK
and finally put the request into listener accept queue is a waste of
bandwidth. We should put the request into listener accept queue and save
one/two messages.
^ permalink raw reply
* Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS
From: Julian Anastasov @ 2012-10-27 8:43 UTC (permalink / raw)
To: Vijay Subramanian
Cc: Eric Dumazet, netdev, davem, edumazet, ncardwell,
Venkat Venkatsubra, Elliott Hughes
In-Reply-To: <CAGK4HS922-xSd0YQdE2kExzMci8A36v_m_GPa17FM7wMJ08n3w@mail.gmail.com>
Hello,
On Fri, 26 Oct 2012, Vijay Subramanian wrote:
> Julian,
> Thanks for the review.
>
> >
> > We have 3 general cases:
> >
> > 1. HTTP-kind of protocol: client sends first, server without
> > TCP_DEFER_ACCEPT
> >
> > Server should retransmit but anyways client will
> > send packet (ACK) that will move request_sock into child socket.
>
> In this case, is this retransmit from server and ack from client
> necessary? I believe that when client finally sends fourth packet i.e.
> data, server socket (currently request_sock) can move into accept
> queue and into ESTABLISHED state. So this patch just removes
> the syn-ack /ack transmissions in between.
I agree that we have the right to stop SYN-ACK
retransmits but only if there is a mechanism that converts
request_sock into child socket and wakeups listener when
there is space for new child. For case 1 this patch works
because it is client's job to talk first. But patch can not
differentiate case 1 from case 3 where child is not created.
> > 2. HTTP-kind of protocol: client sends first, server with
> > TCP_DEFER_ACCEPT
> >
> > Server retransmits before 3WHS to get ACK from
> > client. After ACK we keep request_sock because we do not
> > want to wakeup listener without data. During TCP_DEFER_ACCEPT
> > period nothing is retransmitted because we have ACK from
> > client. After TCP_DEFER_ACCEPT period we start retransmissions
> > because the server needs such request_socks to become
> > child sockets after the TCP_DEFER_ACCEPT period and because
> > received ACK is the only way to create child socket.
> > Server wants to accept() them.
>
> As I mentioned, if client is sending data first, then the data packet
> from client will also cause the creation of child socket.
> If server sends the synack again, if client is not ready with data, it
> will just send an ack back. This is a needless synack/ack.
Indeed, here TCP_DEFER_ACCEPT can also benefit from such
mechanism to convert requests to childs. In this case, child must
appear after the TCP_DEFER_ACCEPT period (in case no data
is received). So, this mechanism should work depending on
the TCP_DEFER_ACCEPT period. We can stop such retransmits
only when new mechanism is implemented.
> On the other hand if client is ready with data, it will send it and
> server socket will create child socket if accept queue has space.
>
> Even with DEFER_ACCEPT sockets, if a third ack comes in without data,
> it is remembered in inet_rsk(req)->acked. So,
> logic is the same. If inet_rsk(req)->acked==1, it means we have
> received third ack and client is in ESTABLISHED state.
> So why bother resending needless synacks?
Only to trigger ACK that will create child. We
just want the child after the TCP_DEFER_ACCEPT period has
expired. Retransmits are the current mechanism to
create child, they are not goal. The goal is to create child.
> > If TCP_DEFER_ACCEPT is
> > above sysctl_tcp_synack_retries there are no such
> > retransmissions because server does not want to accept()
> > such request_socks without data. So, servers decide
> > what they want with the TCP_DEFER_ACCEPT period value.
> >
> > 3. SMTP-kind of protocol: server sends first,
> > TCP_DEFER_ACCEPT must not be used by server.
> > 3WHS is completed, there is no 4th packet from
> > client. It is the server side that needs to move request_sock
> > to child socket, to accept the connection and to send
> > welcome message. AFAIK, child socket is created only on
> > ACK. Or I'm missing something? Now the question is:
>
> You raise a good point. As far as I can see, it looks like the
> request_sock will not become a full socket
> if server is the one that has to send the data first. The problem it
> seems is that we always have to wait
> for the client to send something to create the full socket. Since
> server side already knows that TWHS has finished,
> maybe we can find a way to move a request_sock to accept_queue as
> space opens up without sending synack.
I'm not sure if that is possible, may be ACK comes
with more data that is not saved in request_sock, even
payload. It is lost. From time to time we get new fields
into struct request_sock but I don't know if we have
everything that is needed to create child, I doubt it.
> > how request_sock is moved into child socket if ACK is
> > dropped on syn_recv_sock failure and we do not send SYN-ACK
> > retransmissions to trigger ACK from client? Client does not
> > plan to send new packet in short time.
>
> Agreed. Unless I am missing something too, for cases where server
> socket sends data first, the patch will break things though for
> defer-sockets
> it seems ok. Maybe we need another approach?
The effect of current patch is:
- case 1: saves retransmits, that is good
- case 2: server can not accept child after TCP_DEFER_ACCEPT
period, clients can hate your service. 3WHS is complete
and it is expected longer timers to apply, say 15mins,
while server uses SYN_RECV state timeouts which are
shorter. Client can get RST if data is delayed above
that timers. Without the patch, we will trigger new ACK
from client and new child can enter established state.
TCP_DEFER_ACCEPT period is just an optimization that
avoids wakeups without data for a period but server
still has the following options:
- send SYN-ACK, on ACK create child and send FIN
- send SYN-ACK, on ACK create child and wait 15mins for data
- expire them as request_socks if they do not send data,
later send RST on delayed data from client
With the patch we do not have a way to accept childs.
You restrict server using TCP_DEFER_ACCEPT to
shorter timeout to get client data because we
are left only with the option to expire request_socks,
server can prefer small SYN-ACK retry count but
long timeout for client data.
- case 3: no welcome, client aborts
> Thanks for the feedback. I will wait for suggestions as to what to do
> with the patch and have another look at the code.
I'll think on this problem but my knowledge in this
area is limited. May be the TCP gurus know better what can
be done. My guess is that child creation on ACK is mandatory,
especially when ACK comes with payload.
> Regards,
> Vijay
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* For your interest
From: a_m_yassin@g.pl @ 2012-10-27 7:10 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
--
Greeting,
My name is Mr. A. M. Yassin, I live and work in Abu Dhabi UAE.
I have an urgent business which I believe will interest you.
Find the enclose for details.
For any reason you cannot open that attachment, please let me
know so that I can resend it in the body of the mail, thank you.
I wait for your response, Thank you.
Regards
A. M. Yassin
--
[-- Attachment #2: Interesting deal.pdf --]
[-- Type: application/pdf, Size: 7457 bytes --]
^ permalink raw reply
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