* Re: [PATCH] rxrpc: Replace all unsigned with unsigned int
From: David Howells @ 2016-03-09 23:25 UTC (permalink / raw)
Cc: dhowells, linux-afs, netdev, linux-kernel
In-Reply-To: <20160309232256.18612.76458.stgit@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> wrote:
> Replace all "unsigned" types with "unsigned int" types.
>
> Reported-by: David Miller <davem@davemloft.net>
> Signed-off-by: David Howells <dhowells@redhat.com>
This is aimed at net-next.
David
^ permalink raw reply
* RE: [net PATCH 1/2] e1000: Do not overestimate descriptor counts in Tx pre-check
From: Brown, Aaron F @ 2016-03-09 23:50 UTC (permalink / raw)
To: Alexander Duyck, netdev@vger.kernel.org, jogreene@redhat.com,
intel-wired-lan@lists.osuosl.org, Kirsher, Jeffrey T,
sassmann@redhat.com
In-Reply-To: <20160302211601.2124.64160.stgit@localhost.localdomain>
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Alexander Duyck
> Sent: Wednesday, March 2, 2016 1:16 PM
> To: netdev@vger.kernel.org; jogreene@redhat.com; intel-wired-
> lan@lists.osuosl.org; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sassmann@redhat.com
> Subject: [net PATCH 1/2] e1000: Do not overestimate descriptor counts in Tx
> pre-check
>
> The current code path is capable of grossly overestimating the number of
> descriptors needed to transmit a new frame. This specifically occurs if
> the skb contains a number of 4K pages. The issue is that the logic for
> determining the descriptors needed is ((S) >> (X)) + 1. When X is 12 it
> means that we were indicating that we required 2 descriptors for each 4K
> page when we only needed one.
>
> This change corrects this by instead adding (1 << (X)) - 1 to the S value
> instead of adding 1 after the fact. This way we get an accurate descriptor
> needed count as we are essentially doing a DIV_ROUNDUP().
>
> Reported-by: Ivan Suzdal <isuzdal@mirantis.com>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [net PATCH 2/2] e1000: Double Tx descriptors needed check for 82544
From: Brown, Aaron F @ 2016-03-09 23:51 UTC (permalink / raw)
To: Alexander Duyck, netdev@vger.kernel.org, jogreene@redhat.com,
intel-wired-lan@lists.osuosl.org, Kirsher, Jeffrey T,
sassmann@redhat.com
In-Reply-To: <20160302211608.2124.80148.stgit@localhost.localdomain>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Wednesday, March 2, 2016 1:16 PM
> To: netdev@vger.kernel.org; jogreene@redhat.com; intel-wired-
> lan@lists.osuosl.org; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sassmann@redhat.com
> Subject: [Intel-wired-lan] [net PATCH 2/2] e1000: Double Tx descriptors
> needed check for 82544
>
> The 82544 has code that adds one additional descriptor per data buffer.
> However we weren't taking that into acount when determining the
> descriptors
> needed for the next transmit at the end of the xmit_frame path.
>
> This change takes that into account by doubling the number of descriptors
> needed for the 82544 so that we can avoid a potential issue where we could
> hang the Tx ring by loading frames with xmit_more enabled and then
> stopping
> the ring without writing the tail.
>
> In addition it adds a few more descriptors to account for some additional
> workarounds that have been added over time.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> drivers/net/ethernet/intel/e1000/e1000_main.c | 19
> ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* Re: [PATCH next 3/3] net: Use l3_dev instead of skb->dev for L3 processing
From: Cong Wang @ 2016-03-10 0:09 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Mahesh Bandewar, David Miller, Eric Dumazet, netdev, Tim Hockin,
Alex Pollitt, Matthew Dupre
In-Reply-To: <CAF2d9jh7fbGgibzTgRfNppgVi2tm9j8p6OJHxiaR0ZmGzLe4dg@mail.gmail.com>
On Tue, Mar 8, 2016 at 10:42 AM, Mahesh Bandewar <maheshb@google.com> wrote:
>> The more subsystems involves, the more struct net pointers you
>> potentially need to touch, the less likely you can make it correct
>> by just switching skb->dev.
>
> Please drop that prejudice and read the patch-set carefully. I'm
> neither changing
> any *net* pointers nor changing the skb->dev pointers anywhere. All I'm saying
> is dev->l3_dev is what we'll use for *all* L3 processing and no need to change
> skb->dev or net-ns of any device(s) involved.
Please don't misinterpret me.
I said _switching_, not overwriting or changing, you use dev->l3_dev to _switch_
skb->dev and/or net, this is exactly what I am complaining about. This is not
how netns works.
^ permalink raw reply
* Re: [net-next PATCH] csum: Update csum_block_add to use rotate instead of byteswap
From: Joe Perches @ 2016-03-10 0:18 UTC (permalink / raw)
To: Alexander Duyck, Tom Herbert; +Cc: Alexander Duyck, Netdev, David Miller
In-Reply-To: <CAKgT0UebO62GBsmL17JrZW0Ptzmr05buc1x6pHv6A_PAr4HBLQ@mail.gmail.com>
On Wed, 2016-03-09 at 08:08 -0800, Alexander Duyck wrote:
> On Tue, Mar 8, 2016 at 10:31 PM, Tom Herbert <tom@herbertland.com> wrote:
> > I took a look inlining these.
> >
> > #define rol32(V, X) ({ \
> > int word = V; \
> > if (__builtin_constant_p(X)) \
> > asm("roll $" #X ",%[word]\n\t" \
> > : [word] "=r" (word)); \
> > else \
> > asm("roll %%cl,%[word]\n\t" \
> > : [word] "=r" (word) \
> > : "c" (X)); \
> > word; \
> > })
> >
> > With this I'm seeing a nice speedup in jhash which uses a lot of rol32s...
> Is gcc really not converting the rol32 calls into rotates?
No, it is.
The difference in the object code with the asm for instance is:
(old, compiled with gcc 5.3.1)
<jhash_2words.constprop.5>:
84e: 81 ee 09 41 52 21 sub $0x21524109,%esi
854: 81 ef 09 41 52 21 sub $0x21524109,%edi
85a: 55 push %rbp
85b: 89 f0 mov %esi,%eax
85d: 89 f2 mov %esi,%edx
85f: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 866 <jhash_2words.constprop.5+0x18>
866: c1 c2 0e rol $0xe,%edx
869: 35 f7 be ad de xor $0xdeadbef7,%eax
86e: 48 89 e5 mov %rsp,%rbp
871: 29 d0 sub %edx,%eax
873: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 87a <jhash_2words.constprop.5+0x2c>
87a: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 881 <jhash_2words.constprop.5+0x33>
881: 89 c2 mov %eax,%edx
883: 31 c7 xor %eax,%edi
885: c1 c2 0b rol $0xb,%edx
888: 29 d7 sub %edx,%edi
88a: 89 fa mov %edi,%edx
88c: 31 fe xor %edi,%esi
88e: c1 ca 07 ror $0x7,%edx
891: 29 d6 sub %edx,%esi
893: 89 f2 mov %esi,%edx
895: 31 f0 xor %esi,%eax
897: c1 c2 10 rol $0x10,%edx
89a: 29 d0 sub %edx,%eax
89c: 89 c2 mov %eax,%edx
89e: 31 c7 xor %eax,%edi
8a0: c1 c2 04 rol $0x4,%edx
8a3: 29 d7 sub %edx,%edi
8a5: 31 fe xor %edi,%esi
8a7: c1 c7 0e rol $0xe,%edi
8aa: 29 fe sub %edi,%esi
8ac: 31 f0 xor %esi,%eax
8ae: c1 ce 08 ror $0x8,%esi
8b1: 29 f0 sub %esi,%eax
8b3: 5d pop %rbp
8b4: c3 retq
vs Tom's asm
000000000000084e <jhash_2words.constprop.5>:
84e: 81 ee 09 41 52 21 sub $0x21524109,%esi
854: 8d 87 f7 be ad de lea -0x21524109(%rdi),%eax
85a: 55 push %rbp
85b: 89 f2 mov %esi,%edx
85d: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 864 <jhash_2words.constprop.5+0x16>
864: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 86b <jhash_2words.constprop.5+0x1d>
86b: 81 f2 f7 be ad de xor $0xdeadbef7,%edx
871: 48 89 e5 mov %rsp,%rbp
874: c1 c1 0e rol $0xe,%ecx
877: 29 ca sub %ecx,%edx
879: 31 d0 xor %edx,%eax
87b: c1 c7 0b rol $0xb,%edi
87e: 29 f8 sub %edi,%eax
880: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 887 <jhash_2words.constprop.5+0x39>
887: 31 c6 xor %eax,%esi
889: c1 c7 19 rol $0x19,%edi
88c: 29 fe sub %edi,%esi
88e: 31 f2 xor %esi,%edx
890: c1 c7 10 rol $0x10,%edi
893: 29 fa sub %edi,%edx
895: 31 d0 xor %edx,%eax
897: c1 c7 04 rol $0x4,%edi
89a: 29 f8 sub %edi,%eax
89c: 31 f0 xor %esi,%eax
89e: 29 c8 sub %ecx,%eax
8a0: 31 d0 xor %edx,%eax
8a2: 5d pop %rbp
8a3: c1 c2 18 rol $0x18,%edx
8a6: 29 d0 sub %edx,%eax
8a8: c3 retq
> If we need this type of code in order to get the rotates to occur as
> expected then maybe we need to look at doing arch specific versions of
> the functions in bitops.h in order to improve the performance since I
> know these calls are used in some performance critical paths such as
> crypto and hashing.
Yeah, maybe, but why couldn't gcc generate similar code
as Tom's asm? (modulo the ripple reducing ror vs rol uses
when the shift is > 16
^ permalink raw reply
* Re: [PATCH v6 net-next 0/2] tcp: Redundant Data Bundling (RDB)
From: Yuchung Cheng @ 2016-03-10 0:20 UTC (permalink / raw)
To: Bendik Rønning Opstad
Cc: David S. Miller, netdev, Eric Dumazet, Neal Cardwell,
Andreas Petlund, Carsten Griwodz, Pål Halvorsen,
Jonas Markussen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <1457028388-18226-1-git-send-email-bro.devel+kernel@gmail.com>
On Thu, Mar 3, 2016 at 10:06 AM, Bendik Rønning Opstad
<bro.devel@gmail.com> wrote:
>
> Redundant Data Bundling (RDB) is a mechanism for TCP aimed at reducing
> the latency for applications sending time-dependent data.
> Latency-sensitive applications or services, such as online games and
> remote desktop, produce traffic with thin-stream characteristics,
> characterized by small packets and a relatively high ITT. By bundling
> already sent data in packets with new data, RDB alleviates head-of-line
> blocking by reducing the need to retransmit data segments when packets
> are lost. RDB is a continuation on the work on latency improvements for
> TCP in Linux, previously resulting in two thin-stream mechanisms in the
> Linux kernel
> (https://github.com/torvalds/linux/blob/master/Documentation/networking/tcp-thin.txt).
>
> The RDB implementation has been thoroughly tested, and shows
> significant latency reductions when packet loss occurs[1]. The tests
> show that, by imposing restrictions on the bundling rate, it can be
> made not to negatively affect competing traffic in an unfair manner.
>
> Note: Current patch set depends on the patch "tcp: refactor struct tcp_skb_cb"
> (http://patchwork.ozlabs.org/patch/510674)
>
> These patches have also been tested with as set of packetdrill scripts
> located at
> https://github.com/bendikro/packetdrill/tree/master/gtests/net/packetdrill/tests/linux/rdb
> (The tests require patching packetdrill with a new socket option:
> https://github.com/bendikro/packetdrill/commit/9916b6c53e33dd04329d29b7d8baf703b2c2ac1b)
>
> Detailed info about the RDB mechanism can be found at
> http://mlab.no/blog/2015/10/redundant-data-bundling-in-tcp, as well as
> in the paper "Latency and Fairness Trade-Off for Thin Streams using
> Redundant Data Bundling in TCP"[2].
>
> [1] http://home.ifi.uio.no/paalh/students/BendikOpstad.pdf
> [2] http://home.ifi.uio.no/bendiko/rdb_fairness_tradeoff.pdf
I read the paper. I think the underlying idea is neat. but the
implementation is little heavy-weight that requires changes on fast
path (tcp_write_xmit) and space in skb control blocks. ultimately this
patch is meant for a small set of specific applications.
In my mental model (please correct me if I am wrong), losses on these
thin streams would mostly resort to RTOs instead of fast recovery, due
to the bursty nature of Internet losses. The HOLB comes from RTO only
retransmit the first (tiny) unacked packet while a small of new data is
readily available. But since Linux congestion control is packet-based,
and loss cwnd is 1, the new data needs to wait until the 1st packet is
acked which is for another RTT.
Instead what if we only perform RDB on the (first and recurring) RTO
retransmission?
PS. I don't understand how (old) RDB can masquerade the losses by
skipping DUPACKs. Perhaps an example helps. Suppose we send 4 packets
and the last 3 were (s)acked. We perform RDB to send a packet that has
previous 4 payloads + 1 new byte. The sender still gets the loss
information?
>
> Changes:
>
> v6 (PATCH):
> * tcp-Add-Redundant-Data-Bundling-RDB:
> * Renamed rdb_ack_event() to tcp_rdb_ack_event() (Thanks DaveM)
> * Minor doc changes
>
> * tcp-Add-DPIFL-thin-stream-detection-mechanism:
> * Minor doc changes
>
> v5 (PATCH):
> * tcp-Add-Redundant-Data-Bundling-RDB:
> * Removed two unnecessary EXPORT_SYMOBOLs (Thanks Eric)
> * Renamed skb_append_data() to tcp_skb_append_data() (Thanks Eric)
> * Fixed bugs in additions to ipv4_table (sysctl_net_ipv4.c)
> * Merged the two if tests for max payload of RDB packet in
> rdb_can_bundle_test()
> * Renamed rdb_check_rtx_queue_loss() to rdb_detect_loss()
> and restructured to reduce indentation.
> * Improved docs
> * Revised commit message to be more detailed.
>
> * tcp-Add-DPIFL-thin-stream-detection-mechanism:
> * Fixed bug in additions to ipv4_table (sysctl_net_ipv4.c)
>
> v4 (PATCH):
> * tcp-Add-Redundant-Data-Bundling-RDB:
> * Moved skb_append_data() to tcp_output.c and call this
> function from tcp_collapse_retrans() as well.
> * Merged functionality of create_rdb_skb() into
> tcp_transmit_rdb_skb()
> * Removed one parameter from rdb_can_bundle_test()
>
> v3 (PATCH):
> * tcp-Add-Redundant-Data-Bundling-RDB:
> * Changed name of sysctl variable from tcp_rdb_max_skbs to
> tcp_rdb_max_packets after comment from Eric Dumazet about
> not exposing internal (kernel) names like skb.
> * Formatting and function docs fixes
>
> v2 (RFC/PATCH):
> * tcp-Add-DPIFL-thin-stream-detection-mechanism:
> * Change calculation in tcp_stream_is_thin_dpifl based on
> feedback from Eric Dumazet.
>
> * tcp-Add-Redundant-Data-Bundling-RDB:
> * Removed setting nonagle in do_tcp_setsockopt (TCP_RDB)
> to reduce complexity as commented by Neal Cardwell.
> * Cleaned up loss detection code in rdb_check_rtx_queue_loss
>
> v1 (RFC/PATCH)
>
>
> Bendik Rønning Opstad (2):
> tcp: Add DPIFL thin stream detection mechanism
> tcp: Add Redundant Data Bundling (RDB)
>
> Documentation/networking/ip-sysctl.txt | 23 ++++
> include/linux/skbuff.h | 1 +
> include/linux/tcp.h | 3 +-
> include/net/tcp.h | 36 ++++++
> include/uapi/linux/tcp.h | 1 +
> net/core/skbuff.c | 2 +-
> net/ipv4/Makefile | 3 +-
> net/ipv4/sysctl_net_ipv4.c | 34 +++++
> net/ipv4/tcp.c | 16 ++-
> net/ipv4/tcp_input.c | 3 +
> net/ipv4/tcp_output.c | 48 ++++---
> net/ipv4/tcp_rdb.c | 228 +++++++++++++++++++++++++++++++++
> 12 files changed, 375 insertions(+), 23 deletions(-)
> create mode 100644 net/ipv4/tcp_rdb.c
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH net-next] ovs: allow nl 'flow set' to use ufid without flow key
From: Samuel Gauthier @ 2016-03-10 0:25 UTC (permalink / raw)
To: pravin shelar; +Cc: ovs dev, Linux Kernel Network Developers, David S. Miller
In-Reply-To: <CAOrHB_DtUjG4DkmFZb+Qj1UAWsTOG3sJH0tQEknnTF-PRQCWzQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Sorry, I missed that. Thank you for pointing it out.
Although, set command is also used to reset the flow statistics, and the
action attribute seems optional. Would you find acceptable to make the key
attribute mandatory only if the action attribute is provided?
The ovs-dpctl command could then be simplified to support ovs-dpctl --clear
mod-flow ufid:<ufid> instead of having to specify the flow and action when
you just want to reset the statistics of a flow.
Le 9 mars 2016 10:35 PM, "pravin shelar" <pshelar@ovn.org> a écrit :
> On Wed, Mar 9, 2016 at 9:05 AM, Samuel Gauthier
> <samuel.gauthier@6wind.com> wrote:
> > When we want to change a flow using netlink, we have to identify it to
> > be able to perform a lookup. Both the flow key and unique flow ID
> > (ufid) are valid identifiers, but we always have to specify the flow
> > key in the netlink message. When both attributes are there, the ufid
> > is used.
> >
> > This commit allows to use the ufid without having to provide the flow
> > key, as it is already done in the netlink 'flow get' and 'flow del'
> > path.
> >
> > Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
> > ---
> > net/openvswitch/datapath.c | 21 ++++++++++-----------
> > 1 file changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index deadfdab1bc3..06f15143cf2a 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> > @@ -1100,21 +1100,20 @@ static int ovs_flow_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> > struct sw_flow_match match;
> > struct sw_flow_id sfid;
> > u32 ufid_flags =
> ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> > - int error;
> > + int error = 0;
> > bool log = !a[OVS_FLOW_ATTR_PROBE];
> > bool ufid_present;
> >
> > - /* Extract key. */
> > - error = -EINVAL;
> > - if (!a[OVS_FLOW_ATTR_KEY]) {
> > - OVS_NLERR(log, "Flow key attribute not present in set
> flow.");
> > - goto error;
> > - }
> > -
> > ufid_present = ovs_nla_get_ufid(&sfid, a[OVS_FLOW_ATTR_UFID],
> log);
> > - ovs_match_init(&match, &key, &mask);
> > - error = ovs_nla_get_match(net, &match, a[OVS_FLOW_ATTR_KEY],
> > - a[OVS_FLOW_ATTR_MASK], log);
> > + if (a[OVS_FLOW_ATTR_KEY]) {
> > + ovs_match_init(&match, &key, NULL);
> > + error = ovs_nla_get_match(net, &match,
> a[OVS_FLOW_ATTR_KEY],
> > + NULL, log);
> > + } else if (!ufid_present) {
> > + OVS_NLERR(log,
> > + "Flow set message rejected, Key attribute
> missing.");
> > + error = -EINVAL;
> > + }
>
> Set command sets new action for given flow. The action validation
> depends on the flow key and mask. so userspace needs to provide the
> key and mask.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next] ovs: allow nl 'flow set' to use ufid without flow key
From: pravin shelar @ 2016-03-10 0:32 UTC (permalink / raw)
To: Samuel Gauthier; +Cc: ovs dev, Linux Kernel Network Developers, David S. Miller
In-Reply-To: <CAMEOZhK0kBpdS8uOqYqzQs+6LyWh5K3MWL__CRoThmNRnBN2MQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Mar 9, 2016 at 4:25 PM, Samuel Gauthier
<samuel.gauthier@6wind.com> wrote:
> Sorry, I missed that. Thank you for pointing it out.
>
> Although, set command is also used to reset the flow statistics, and the
> action attribute seems optional. Would you find acceptable to make the key
> attribute mandatory only if the action attribute is provided?
>
> The ovs-dpctl command could then be simplified to support ovs-dpctl --clear
> mod-flow ufid:<ufid> instead of having to specify the flow and action when
> you just want to reset the statistics of a flow.
>
That is fine with me.
Thanks,
Pravin.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [net-next PATCH] csum: Update csum_block_add to use rotate instead of byteswap
From: Tom Herbert @ 2016-03-10 0:58 UTC (permalink / raw)
To: Joe Perches; +Cc: Alexander Duyck, Alexander Duyck, Netdev, David Miller
In-Reply-To: <1457569113.3433.7.camel@perches.com>
On Wed, Mar 9, 2016 at 4:18 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2016-03-09 at 08:08 -0800, Alexander Duyck wrote:
>> On Tue, Mar 8, 2016 at 10:31 PM, Tom Herbert <tom@herbertland.com> wrote:
>> > I took a look inlining these.
>> >
>> > #define rol32(V, X) ({ \
>> > int word = V; \
>> > if (__builtin_constant_p(X)) \
>> > asm("roll $" #X ",%[word]\n\t" \
>> > : [word] "=r" (word)); \
>> > else \
>> > asm("roll %%cl,%[word]\n\t" \
>> > : [word] "=r" (word) \
>> > : "c" (X)); \
>> > word; \
>> > })
>> >
>> > With this I'm seeing a nice speedup in jhash which uses a lot of rol32s...
>> Is gcc really not converting the rol32 calls into rotates?
>
> No, it is.
>
> The difference in the object code with the asm for instance is:
>
> (old, compiled with gcc 5.3.1)
>
> <jhash_2words.constprop.5>:
> 84e: 81 ee 09 41 52 21 sub $0x21524109,%esi
> 854: 81 ef 09 41 52 21 sub $0x21524109,%edi
> 85a: 55 push %rbp
> 85b: 89 f0 mov %esi,%eax
> 85d: 89 f2 mov %esi,%edx
> 85f: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 866 <jhash_2words.constprop.5+0x18>
> 866: c1 c2 0e rol $0xe,%edx
> 869: 35 f7 be ad de xor $0xdeadbef7,%eax
> 86e: 48 89 e5 mov %rsp,%rbp
> 871: 29 d0 sub %edx,%eax
> 873: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 87a <jhash_2words.constprop.5+0x2c>
> 87a: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 881 <jhash_2words.constprop.5+0x33>
> 881: 89 c2 mov %eax,%edx
> 883: 31 c7 xor %eax,%edi
> 885: c1 c2 0b rol $0xb,%edx
> 888: 29 d7 sub %edx,%edi
> 88a: 89 fa mov %edi,%edx
> 88c: 31 fe xor %edi,%esi
> 88e: c1 ca 07 ror $0x7,%edx
> 891: 29 d6 sub %edx,%esi
> 893: 89 f2 mov %esi,%edx
> 895: 31 f0 xor %esi,%eax
> 897: c1 c2 10 rol $0x10,%edx
> 89a: 29 d0 sub %edx,%eax
> 89c: 89 c2 mov %eax,%edx
> 89e: 31 c7 xor %eax,%edi
> 8a0: c1 c2 04 rol $0x4,%edx
> 8a3: 29 d7 sub %edx,%edi
> 8a5: 31 fe xor %edi,%esi
> 8a7: c1 c7 0e rol $0xe,%edi
> 8aa: 29 fe sub %edi,%esi
> 8ac: 31 f0 xor %esi,%eax
> 8ae: c1 ce 08 ror $0x8,%esi
> 8b1: 29 f0 sub %esi,%eax
> 8b3: 5d pop %rbp
> 8b4: c3 retq
>
> vs Tom's asm
>
> 000000000000084e <jhash_2words.constprop.5>:
> 84e: 81 ee 09 41 52 21 sub $0x21524109,%esi
> 854: 8d 87 f7 be ad de lea -0x21524109(%rdi),%eax
> 85a: 55 push %rbp
> 85b: 89 f2 mov %esi,%edx
> 85d: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 864 <jhash_2words.constprop.5+0x16>
> 864: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 86b <jhash_2words.constprop.5+0x1d>
> 86b: 81 f2 f7 be ad de xor $0xdeadbef7,%edx
> 871: 48 89 e5 mov %rsp,%rbp
> 874: c1 c1 0e rol $0xe,%ecx
> 877: 29 ca sub %ecx,%edx
> 879: 31 d0 xor %edx,%eax
> 87b: c1 c7 0b rol $0xb,%edi
> 87e: 29 f8 sub %edi,%eax
> 880: 48 ff 05 00 00 00 00 incq 0x0(%rip) # 887 <jhash_2words.constprop.5+0x39>
> 887: 31 c6 xor %eax,%esi
> 889: c1 c7 19 rol $0x19,%edi
> 88c: 29 fe sub %edi,%esi
> 88e: 31 f2 xor %esi,%edx
> 890: c1 c7 10 rol $0x10,%edi
> 893: 29 fa sub %edi,%edx
> 895: 31 d0 xor %edx,%eax
> 897: c1 c7 04 rol $0x4,%edi
> 89a: 29 f8 sub %edi,%eax
> 89c: 31 f0 xor %esi,%eax
> 89e: 29 c8 sub %ecx,%eax
> 8a0: 31 d0 xor %edx,%eax
> 8a2: 5d pop %rbp
> 8a3: c1 c2 18 rol $0x18,%edx
> 8a6: 29 d0 sub %edx,%eax
> 8a8: c3 retq
>
>> If we need this type of code in order to get the rotates to occur as
>> expected then maybe we need to look at doing arch specific versions of
>> the functions in bitops.h in order to improve the performance since I
>> know these calls are used in some performance critical paths such as
>> crypto and hashing.
>
> Yeah, maybe, but why couldn't gcc generate similar code
> as Tom's asm? (modulo the ripple reducing ror vs rol uses
> when the shift is > 16
I see gcc doing that now, not sure why I was seeing differences before....
^ permalink raw reply
* Re: [PATCH net-next v3] tcp: Add RFC4898 tcpEStatsPerfDataSegsOut/In
From: Martin KaFai Lau @ 2016-03-10 1:55 UTC (permalink / raw)
To: Yuchung Cheng
Cc: netdev, Kernel Team, Chris Rapier, Eric Dumazet,
Marcelo Ricardo Leitner, Neal Cardwell
In-Reply-To: <CAK6E8=fxc+uefYnnEb1Np7OUTNASXvYeOfJOtEeABdWOqEY_=w@mail.gmail.com>
On Wed, Mar 09, 2016 at 03:11:50PM -0800, Yuchung Cheng wrote:
> On Wed, Mar 9, 2016 at 10:43 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index e90db85..24557a8 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -1816,4 +1816,14 @@ static inline void skb_set_tcp_pure_ack(struct sk_buff *skb)
> > skb->truesize = 2;
> > }
> >
> > +static inline void tcp_segs_in(struct tcp_sock *tp, const struct sk_buff *skb)
> > +{
> > + u16 segs_in;
> > +
> > + segs_in = max_t(u16, 1, skb_shinfo(skb)->gso_segs);
> > + tp->segs_in += segs_in;
> > + if (skb->len > tcp_hdrlen(skb))
> > + tp->data_segs_in += segs_in;
> > +}
> > +
> > #endif /* _TCP_H */
> > diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> > index fdb286d..f583c85 100644
> > --- a/net/ipv4/tcp_fastopen.c
> > +++ b/net/ipv4/tcp_fastopen.c
> > @@ -131,6 +131,7 @@ static bool tcp_fastopen_cookie_gen(struct request_sock *req,
> > void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
> > {
> > struct tcp_sock *tp = tcp_sk(sk);
> > + u16 segs_in;
> >
> > if (TCP_SKB_CB(skb)->end_seq == tp->rcv_nxt)
> > return;
> > @@ -154,6 +155,9 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
> > * as we certainly are not changing upper 32bit value (0)
> > */
> > tp->bytes_received = skb->len;
> > + segs_in = max_t(u16, 1, skb_shinfo(skb)->gso_segs);
> > + tp->segs_in = segs_in;
> > + tp->data_segs_in = segs_in;
> why not use the new tcp_segs_in() helper?
It is because
segs_in has been set to 1 by tcp_create_openreq_child(), so calling
tcp_segs_in() will have it double counted
and
tcphdr has been pulled from skb, so skb->len does not include tcp_hdrlen.
I will add some remark in this change.
^ permalink raw reply
* [patch net 1/2] net: hns: fix return value of the function about rss
From: Kejian Yan @ 2016-03-10 2:16 UTC (permalink / raw)
To: davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd,
andriy.shevchenko, andrew, chenny.xu, ivecera, lisheng011,
fengguang.wu, haifeng.wei, netdev, linux-kernel, linux-arm-kernel,
linuxarm
In-Reply-To: <1457576189-22924-1-git-send-email-yankejian@huawei.com>
Both .get_rxfh and .get_rxfh are always return 0, it should return result
from hardware when getting or setting rss. And the rss function should
return the correct data type.
Signed-off-by: Kejian Yan <yankejian@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 8 ++++----
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index a0070d0..3b8f301 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -791,7 +791,7 @@ static int hns_ae_set_rss(struct hnae_handle *handle, const u32 *indir,
/* set the RSS Hash Key if specififed by the user */
if (key)
- hns_ppe_set_rss_key(ppe_cb, (int *)key);
+ hns_ppe_set_rss_key(ppe_cb, (u32 *)key);
/* update the shadow RSS table with user specified qids */
memcpy(ppe_cb->rss_indir_table, indir, HNS_PPEV2_RSS_IND_TBL_SIZE);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index f302ef9..811ef35 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -27,7 +27,7 @@ void hns_ppe_set_tso_enable(struct hns_ppe_cb *ppe_cb, u32 value)
void hns_ppe_set_rss_key(struct hns_ppe_cb *ppe_cb,
const u32 rss_key[HNS_PPEV2_RSS_KEY_NUM])
{
- int key_item = 0;
+ u32 key_item = 0;
for (key_item = 0; key_item < HNS_PPEV2_RSS_KEY_NUM; key_item++)
dsaf_write_dev(ppe_cb, PPEV2_RSS_KEY_REG + key_item * 0x4,
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 3df2284..ada8e04 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -1165,7 +1165,7 @@ hns_get_rss_key_size(struct net_device *netdev)
if (AE_IS_VER1(priv->enet_ver)) {
netdev_err(netdev,
"RSS feature is not supported on this hardware\n");
- return -EOPNOTSUPP;
+ return (u32)-EOPNOTSUPP;
}
ops = priv->ae_handle->dev->ops;
@@ -1184,7 +1184,7 @@ hns_get_rss_indir_size(struct net_device *netdev)
if (AE_IS_VER1(priv->enet_ver)) {
netdev_err(netdev,
"RSS feature is not supported on this hardware\n");
- return -EOPNOTSUPP;
+ return (u32)-EOPNOTSUPP;
}
ops = priv->ae_handle->dev->ops;
@@ -1213,7 +1213,7 @@ hns_get_rss(struct net_device *netdev, u32 *indir, u8 *key, u8 *hfunc)
ret = ops->get_rss(priv->ae_handle, indir, key, hfunc);
- return 0;
+ return ret;
}
static int
@@ -1241,7 +1241,7 @@ hns_set_rss(struct net_device *netdev, const u32 *indir, const u8 *key,
ret = ops->set_rss(priv->ae_handle, indir, key, hfunc);
- return 0;
+ return ret;
}
static struct ethtool_ops hns_ethtool_ops = {
--
1.9.1
^ permalink raw reply related
* [patch net 0/2] net: hns: get and set RSS indirection table by using ethtool
From: Kejian Yan @ 2016-03-10 2:16 UTC (permalink / raw)
To: davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd,
andriy.shevchenko, andrew, chenny.xu, ivecera, lisheng011,
fengguang.wu, haifeng.wei, netdev, linux-kernel, linux-arm-kernel,
linuxarm
When we use ethtool to retrieves or configure the receive flow hash
indirection table, ethtool needs to call .get_rxnfc to get the ring number
so this patchset implements the .get_rxnfc and fixes the bug that we can
not get the tatal table each time.
Kejian Yan (2):
net: hns: fix return value of the function about rss
net: hns: fixes a bug of RSS
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 8 ++++---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 28 +++++++++++++++++++----
3 files changed, 30 insertions(+), 8 deletions(-)
--
1.9.1
^ permalink raw reply
* [patch net 2/2] net: hns: fixes a bug of RSS
From: Kejian Yan @ 2016-03-10 2:16 UTC (permalink / raw)
To: davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd,
andriy.shevchenko, andrew, chenny.xu, ivecera, lisheng011,
fengguang.wu, haifeng.wei, netdev, linux-kernel, linux-arm-kernel,
linuxarm
In-Reply-To: <1457576189-22924-1-git-send-email-yankejian@huawei.com>
If trying to get receive flow hash indirection table by ethtool, it needs
to call .get_rxnfc to get ring number first. So this patch implements the
.get_rxnfc of ethtool. And the data type of rss_indir_table is u32, it has
to be multiply by the width of data type when using memcpy.
Signed-off-by: Kejian Yan <yankejian@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 6 ++++--
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index 3b8f301..c733a5a 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -779,7 +779,8 @@ static int hns_ae_get_rss(struct hnae_handle *handle, u32 *indir, u8 *key,
memcpy(key, ppe_cb->rss_key, HNS_PPEV2_RSS_KEY_SIZE);
/* update the current hash->queue mappings from the shadow RSS table */
- memcpy(indir, ppe_cb->rss_indir_table, HNS_PPEV2_RSS_IND_TBL_SIZE);
+ memcpy(indir, ppe_cb->rss_indir_table,
+ HNS_PPEV2_RSS_IND_TBL_SIZE * sizeof(*indir));
return 0;
}
@@ -794,7 +795,8 @@ static int hns_ae_set_rss(struct hnae_handle *handle, const u32 *indir,
hns_ppe_set_rss_key(ppe_cb, (u32 *)key);
/* update the shadow RSS table with user specified qids */
- memcpy(ppe_cb->rss_indir_table, indir, HNS_PPEV2_RSS_IND_TBL_SIZE);
+ memcpy(ppe_cb->rss_indir_table, indir,
+ HNS_PPEV2_RSS_IND_TBL_SIZE * sizeof(*indir));
/* now update the hardware */
hns_ppe_set_indir_table(ppe_cb, ppe_cb->rss_indir_table);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index ada8e04..a070392 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -1244,6 +1244,25 @@ hns_set_rss(struct net_device *netdev, const u32 *indir, const u8 *key,
return ret;
}
+static int hns_get_rxnfc(struct net_device *netdev,
+ struct ethtool_rxnfc *cmd,
+ u32 *rule_locs)
+{
+ struct hns_nic_priv *priv = netdev_priv(netdev);
+ int ret = 0;
+
+ switch (cmd->cmd) {
+ case ETHTOOL_GRXRINGS:
+ cmd->data = priv->ae_handle->q_num;
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ return ret;
+}
+
static struct ethtool_ops hns_ethtool_ops = {
.get_drvinfo = hns_nic_get_drvinfo,
.get_link = hns_nic_get_link,
@@ -1267,6 +1286,7 @@ static struct ethtool_ops hns_ethtool_ops = {
.get_rxfh_indir_size = hns_get_rss_indir_size,
.get_rxfh = hns_get_rss,
.set_rxfh = hns_set_rss,
+ .get_rxnfc = hns_get_rxnfc,
};
void hns_ethtool_set_ops(struct net_device *ndev)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v6 net-next 0/2] tcp: Redundant Data Bundling (RDB)
From: Jonas Markussen @ 2016-03-10 1:45 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Bendik Rønning Opstad, David S. Miller, netdev, Eric Dumazet,
Neal Cardwell, Andreas Petlund, Carsten Griwodz,
Pål Halvorsen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <CAK6E8=eKu_0ReSDLYdyajxMyX2MaN=wDLHvq=u_tyNnMRS31sw@mail.gmail.com>
> On 10 Mar 2016, at 01:20, Yuchung Cheng <ycheng@google.com> wrote:
>
> PS. I don't understand how (old) RDB can masquerade the losses by
> skipping DUPACKs. Perhaps an example helps. Suppose we send 4 packets
> and the last 3 were (s)acked. We perform RDB to send a packet that has
> previous 4 payloads + 1 new byte. The sender still gets the loss
> information?
>
If I’ve understood you correctly, you’re talking about sending 4
packets and the first one is lost?
In this case, RDB will not only bundle on the last/new packet but also
as it sends packet 2 (which will contain 1+2), packet 3 (1+2+3)
and packet 4 (1+2+3+4).
So the fact that packet 1 was lost is masqueraded when it is
recovered by packet 2 and there won’t be any gap in the SACK window
indicating that packet 1 was lost.
Best regards,
Jonas Markussen
^ permalink raw reply
* Re: [PATCH nf-next v9 4/8] openvswitch: Update the CT state key only after nf_conntrack_in().
From: Joe Stringer @ 2016-03-10 2:12 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: ovs dev, netdev, netfilter-devel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457565024-27300-4-git-send-email-jarno-LZ6Gd1LRuIk@public.gmane.org>
On 9 March 2016 at 15:10, Jarno Rajahalme <jarno@ovn.org> wrote:
> Only a successful nf_conntrack_in() call can effect a connection state
> change, so if suffices to update the key only after the
> nf_conntrack_in() returns.
"it" suffices to update...
> This change is needed for the later NAT patches.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Joe Stringer <joe@ovn.org>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH net] net: hns: bug fix about ping6
From: Kejian Yan @ 2016-03-10 2:17 UTC (permalink / raw)
To: davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd,
andriy.shevchenko, andrew, chenny.xu, ivecera, lisheng011,
fengguang.wu, haifeng.wei, netdev, linux-kernel, linux-arm-kernel,
linuxarm
The current upstreaming code fails to ping other IPv6 net device, because
the enet receives the multicast packets with the src mac addr whick is the
same as its mac addr. These packets need to be dropped.
Signed-off-by: Kejian Yan <yankejian@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 3f77ff7..9ad5da4 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -564,6 +564,7 @@ static int hns_nic_poll_rx_skb(struct hns_nic_ring_data *ring_data,
struct sk_buff *skb;
struct hnae_desc *desc;
struct hnae_desc_cb *desc_cb;
+ struct ethhdr *eh;
unsigned char *va;
int bnum, length, i;
int pull_len;
@@ -670,6 +671,14 @@ out_bnum_err:
return -EFAULT;
}
+ /* filter out multicast pkt with the same src mac as this port */
+ eh = (struct ethhdr *)skb->data;
+ if (unlikely(is_multicast_ether_addr(eh->h_dest) &&
+ ether_addr_equal(ndev->dev_addr, eh->h_source))) {
+ dev_kfree_skb_any(skb);
+ return -EFAULT;
+ }
+
ring->stats.rx_pkts++;
ring->stats.rx_bytes += skb->len;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v6 net-next 0/2] tcp: Redundant Data Bundling (RDB)
From: Yuchung Cheng @ 2016-03-10 2:27 UTC (permalink / raw)
To: Jonas Markussen
Cc: Bendik Rønning Opstad, David S. Miller, netdev, Eric Dumazet,
Neal Cardwell, Andreas Petlund, Carsten Griwodz,
Pål Halvorsen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <78983A17-6B0C-4A11-BE0E-A98FE699D1E3@ifi.uio.no>
On Wed, Mar 9, 2016 at 5:45 PM, Jonas Markussen <jonassm@ifi.uio.no> wrote:
>
>> On 10 Mar 2016, at 01:20, Yuchung Cheng <ycheng@google.com> wrote:
>>
>> PS. I don't understand how (old) RDB can masquerade the losses by
>> skipping DUPACKs. Perhaps an example helps. Suppose we send 4 packets
>> and the last 3 were (s)acked. We perform RDB to send a packet that has
>> previous 4 payloads + 1 new byte. The sender still gets the loss
>> information?
>>
>
> If I’ve understood you correctly, you’re talking about sending 4
> packets and the first one is lost?
>
> In this case, RDB will not only bundle on the last/new packet but also
> as it sends packet 2 (which will contain 1+2), packet 3 (1+2+3)
> and packet 4 (1+2+3+4).
>
> So the fact that packet 1 was lost is masqueraded when it is
> recovered by packet 2 and there won’t be any gap in the SACK window
> indicating that packet 1 was lost.
I see. Thanks for the clarification.
So my question is still if thin-stream app has enough inflight to use
ack-triggered recovery. i.e., it has to send at least twice within an
RTT.
Also have you tested this with non-Linux receivers? Thanks.
>
> Best regards,
> Jonas Markussen
^ permalink raw reply
* Re: [PATCH nf-next v9 5/8] openvswitch: Find existing conntrack entry after upcall.
From: Joe Stringer @ 2016-03-10 2:29 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: ovs dev, netdev, netfilter-devel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457565024-27300-5-git-send-email-jarno-LZ6Gd1LRuIk@public.gmane.org>
On 9 March 2016 at 15:10, Jarno Rajahalme <jarno@ovn.org> wrote:
> Add a new function ovs_ct_find_existing() to find an existing
> conntrack entry for which this packet was already applied to. This is
> only to be called when there is evidence that the packet was already
> tracked and committed, but we lost the ct reference due to an
> userspace upcall.
>
> ovs_ct_find_existing() is called from skb_nfct_cached(), which can now
> hide the fact that the ct reference may have been lost due to an
> upcall. This allows ovs_ct_commit() to be simplified.
>
> This patch is needed by later "openvswitch: Interface with NAT" patch,
> as we need to be able to pass the packet through NAT using the
> original ct reference also after the reference is lost after an
> upcall.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Joe Stringer <joe@ovn.org>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH nf-next v9 7/8] openvswitch: Delay conntrack helper call for new connections.
From: Joe Stringer @ 2016-03-10 2:37 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: ovs dev, netdev, netfilter-devel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457565024-27300-7-git-send-email-jarno-LZ6Gd1LRuIk@public.gmane.org>
On 9 March 2016 at 15:10, Jarno Rajahalme <jarno@ovn.org> wrote:
> There is no need to help connections that are not confirmed, so we can
> delay helping new connections to the time when they are confirmed.
> This change is needed for NAT support, and having this as a separate
> patch will make the following NAT patch a bit easier to review.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> net/openvswitch/conntrack.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 92613de..5711f80 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
<snip>
> @@ -506,11 +510,17 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
> return -ENOENT;
>
> ovs_ct_update_key(skb, info, key, true);
> + }
>
> - if (ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
> - WARN_ONCE(1, "helper rejected packet");
> - return -EINVAL;
> - }
> + /* Call the helper only if we did nf_conntrack_in() above ('!cached')
> + * for confirmed connections, but only when committing for unconfirmed
> + * connections.
> + */
Minor nit, try this wording for readibility?
/* Call the helper only if:
* - nf_conntrack_in() was executed above ("!cached") for a confirmed
connection, or
* - When committing an unconfirmed connection
*/
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [RFC PATCH 1/1] net namespace: dynamically configure new net namespace inherit net config
From: Zhu Yanjun @ 2016-03-10 2:54 UTC (permalink / raw)
To: zyjzyj2000, davem, kuznet, jmorris, yoshfuji, kaber, netdev,
bruce.ashfield
Sometimes the system engineer and application expect a new net namespace
to inherit config from the base net config. Sometimes the current net config
is expected by the system engineer and application. So it is necessary that
the system engineer and application can choose a new net namespace to inherit
from the base net config, or the current net config.
For example, the value of /proc/sys/net/ipv4/ip_forward is taken as
an example. The value of /proc/sys/net/ipv4/ip_forward in the base net
config is 0 while the value of /proc/sys/net/ipv4/ip_forward is changed
to 1 in the current net config. The system engineer and application can choose
a new net namespace to inherit the value of /proc/sys/net/ipv4/ip_forward from
the base or the current settings.
Test case:
1. % cat /proc/sys/net/ipv4/net_ns_inherit
1
2. Set ip forwarding in the "base namespace"
% echo 1 > /proc/sys/net/ipv4/ip_forward
% cat /proc/sys/net/ipv4/ip_forward
1
3. Create a new namespace
% ip netns add mynewns
4. Check ip forwarding in the new namespace
% ip netns exec mynewns cat /proc/sys/net/ipv4/ip_forward
1
5. % echo 0 > /proc/sys/net/ipv4/net_ns_inherit
% cat /proc/sys/net/ipv4/net_ns_inherit
0
6. Set ip forwarding in the "base namespace"
% echo 1 > /proc/sys/net/ipv4/ip_forward
% cat /proc/sys/net/ipv4/ip_forward
1
7. Create a new namespace
% ip netns add mynewns_new
8. Check ip forwarding in the new namespace
% ip netns exec mynewns_new cat /proc/sys/net/ipv4/ip_forward
0
Suggested-by: Bruce Ashfield <bruce.ashfield@windriver.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
CC: David S. Miller <davem@davemloft.net>
CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
CC: James Morris <jmorris@namei.org>
CC: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
CC: Patrick McHardy <kaber@trash.net>
---
include/linux/inetdevice.h | 2 +-
include/net/ip.h | 3 +++
include/uapi/linux/sysctl.h | 1 +
net/ipv4/devinet.c | 58 ++++++++++++++++++++++++++++++++++++-------
net/ipv4/sysctl_net_ipv4.c | 7 ++++++
5 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ee971f3..1c0ae93 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -164,7 +164,7 @@ static inline struct net_device *ip_dev_find(struct net *net, __be32 addr)
int inet_addr_onlink(struct in_device *in_dev, __be32 a, __be32 b);
int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
-void devinet_init(void);
+int devinet_init(void);
struct in_device *inetdev_by_index(struct net *, int);
__be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
diff --git a/include/net/ip.h b/include/net/ip.h
index 1a98f1c..0ad4a7d 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -245,6 +245,9 @@ extern int inet_peer_threshold;
extern int inet_peer_minttl;
extern int inet_peer_maxttl;
+/* From devinet.c */
+extern int net_ns_inherit;
+
/* From ip_input.c */
extern int sysctl_ip_early_demux;
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 0956373..350c3ce 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -426,6 +426,7 @@ enum
NET_TCP_ALLOWED_CONG_CONTROL=123,
NET_TCP_MAX_SSTHRESH=124,
NET_TCP_FRTO_RESPONSE=125,
+ NET_IPV4_NET_NS_INHERIT = 126,
};
enum {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index cebd9d3..b68d7fa 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2277,28 +2277,31 @@ static struct ctl_table ctl_forward_entry[] = {
};
#endif
+#define NET_NS_INIT_DEFAULT 0
+#define NET_NS_INIT_MODIFIED 1
+
+/* net ns initialized from current */
+int net_ns_inherit __read_mostly = NET_NS_INIT_MODIFIED;
+static struct ipv4_devconf *all_backup, *dflt_backup;
+
static __net_init int devinet_init_net(struct net *net)
{
int err;
- struct ipv4_devconf *all, *dflt;
+ struct ipv4_devconf *all = NULL, *dflt = NULL;
#ifdef CONFIG_SYSCTL
struct ctl_table *tbl = ctl_forward_entry;
struct ctl_table_header *forw_hdr;
#endif
-
err = -ENOMEM;
- all = &ipv4_devconf;
- dflt = &ipv4_devconf_dflt;
- if (!net_eq(net, &init_net)) {
- all = kmemdup(all, sizeof(ipv4_devconf), GFP_KERNEL);
+ if (net_ns_inherit == NET_NS_INIT_DEFAULT) {
+ all = kmemdup(all_backup, sizeof(ipv4_devconf), GFP_KERNEL);
if (!all)
goto err_alloc_all;
- dflt = kmemdup(dflt, sizeof(ipv4_devconf_dflt), GFP_KERNEL);
+ dflt = kmemdup(dflt_backup, sizeof(ipv4_devconf_dflt), GFP_KERNEL);
if (!dflt)
goto err_alloc_dflt;
-
#ifdef CONFIG_SYSCTL
tbl = kmemdup(tbl, sizeof(ctl_forward_entry), GFP_KERNEL);
if (!tbl)
@@ -2309,6 +2312,29 @@ static __net_init int devinet_init_net(struct net *net)
tbl[0].extra2 = net;
#endif
}
+ if (net_ns_inherit == NET_NS_INIT_MODIFIED) {
+ all = &ipv4_devconf;
+ dflt = &ipv4_devconf_dflt;
+
+ if (!net_eq(net, &init_net)) {
+ all = kmemdup(all, sizeof(ipv4_devconf), GFP_KERNEL);
+ if (!all)
+ goto err_alloc_all;
+
+ dflt = kmemdup(dflt, sizeof(ipv4_devconf_dflt), GFP_KERNEL);
+ if (!dflt)
+ goto err_alloc_dflt;
+#ifdef CONFIG_SYSCTL
+ tbl = kmemdup(tbl, sizeof(ctl_forward_entry), GFP_KERNEL);
+ if (!tbl)
+ goto err_alloc_ctl;
+
+ tbl[0].data = &all->data[IPV4_DEVCONF_FORWARDING - 1];
+ tbl[0].extra1 = all;
+ tbl[0].extra2 = net;
+#endif
+ }
+ }
#ifdef CONFIG_SYSCTL
err = __devinet_sysctl_register(net, "all", all);
@@ -2360,6 +2386,8 @@ static __net_exit void devinet_exit_net(struct net *net)
__devinet_sysctl_unregister(net->ipv4.devconf_all);
kfree(tbl);
#endif
+ kfree(all_backup);
+ kfree(dflt_backup);
kfree(net->ipv4.devconf_dflt);
kfree(net->ipv4.devconf_all);
}
@@ -2377,10 +2405,20 @@ static struct rtnl_af_ops inet_af_ops __read_mostly = {
.set_link_af = inet_set_link_af,
};
-void __init devinet_init(void)
+int __init devinet_init(void)
{
int i;
+ all_backup = kmemdup(&ipv4_devconf, sizeof(ipv4_devconf), GFP_KERNEL);
+ if (!all_backup) {
+ return -ENOBUFS;
+ }
+
+ dflt_backup = kmemdup(&ipv4_devconf_dflt, sizeof(ipv4_devconf_dflt), GFP_KERNEL);
+ if (!dflt_backup) {
+ return -ENOBUFS;
+ }
+
for (i = 0; i < IN4_ADDR_HSIZE; i++)
INIT_HLIST_HEAD(&inet_addr_lst[i]);
@@ -2398,4 +2436,6 @@ void __init devinet_init(void)
rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr, NULL);
rtnl_register(PF_INET, RTM_GETNETCONF, inet_netconf_get_devconf,
inet_netconf_dump_devconf, NULL);
+
+ return 0;
}
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index a0bd7a5..d4a68e3 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -799,6 +799,13 @@ static struct ctl_table ipv4_table[] = {
.proc_handler = proc_dointvec_minmax,
.extra1 = &one
},
+ {
+ .procname = "net_ns_inherit",
+ .data = &net_ns_inherit,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
{ }
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next] bpf: bpf_stackmap_copy depends on CONFIG_PERF_EVENTS
From: Alexei Starovoitov @ 2016-03-10 2:56 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, Fengguang Wu, netdev
0-day bot reported build error:
kernel/built-in.o: In function `map_lookup_elem':
>> kernel/bpf/.tmp_syscall.o:(.text+0x329b3c): undefined reference to `bpf_stackmap_copy'
when CONFIG_BPF_SYSCALL is set and CONFIG_PERF_EVENTS is not.
Add weak definition to resolve it.
This code path in map_lookup_elem() is never taken
when CONFIG_PERF_EVENTS is not set.
Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/syscall.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2978d0d08869..2a2efe1bc76c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -244,6 +244,11 @@ static void __user *u64_to_ptr(__u64 val)
return (void __user *) (unsigned long) val;
}
+int __weak bpf_stackmap_copy(struct bpf_map *map, void *key, void *value)
+{
+ return -ENOTSUPP;
+}
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD value
--
2.8.0.rc1
^ permalink raw reply related
* [PATCH net-next 0/3] validate variable length ll headers
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Allow device-specific validation of link layer headers. Existing
checks drop all packets shorter than hard_header_len. For variable
length protocols, such packets can be valid.
patch 1 adds header_ops.validate and dev_validate_header
patch 2 implements the protocol specific callback for AX25
patch 3 replaces ll_header_truncated with dev_validate_header
Willem de Bruijn (3):
net: validate variable length ll headers
ax25: add link layer header validation function
packet: validate variable length ll headers
include/linux/netdevice.h | 22 ++++++++++++++++++++--
net/ax25/ax25_ip.c | 15 +++++++++++++++
net/packet/af_packet.c | 43 ++++++++++++++++++-------------------------
3 files changed, 53 insertions(+), 27 deletions(-)
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply
* [PATCH net-next 1/3] net: validate variable length ll headers
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
Netdevice parameter hard_header_len is variously interpreted both as
an upper and lower bound on link layer header length. The field is
used as upper bound when reserving room at allocation, as lower bound
when validating user input in PF_PACKET.
Clarify the definition to be maximum header length. For validation
of untrusted headers, add an optional validate member to header_ops.
Allow bypassing of validation by passing CAP_SYS_RAWIO, for instance
for deliberate testing of corrupt input. In this case, pad trailing
bytes, as some device drivers expect completely initialized headers.
See also http://comments.gmane.org/gmane.linux.network/401064
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/netdevice.h | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index efe7cec..fd30cb5 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -268,6 +268,7 @@ struct header_ops {
void (*cache_update)(struct hh_cache *hh,
const struct net_device *dev,
const unsigned char *haddr);
+ bool (*validate)(const char *ll_header, unsigned int len);
};
/* These flag bits are private to the generic network queueing
@@ -1459,8 +1460,7 @@ enum netdev_priv_flags {
* @dma: DMA channel
* @mtu: Interface MTU value
* @type: Interface hardware type
- * @hard_header_len: Hardware header length, which means that this is the
- * minimum size of a packet.
+ * @hard_header_len: Maximum hardware header length.
*
* @needed_headroom: Extra headroom the hardware may need, but not in all
* cases can this be guaranteed
@@ -2687,6 +2687,24 @@ static inline int dev_parse_header(const struct sk_buff *skb,
return dev->header_ops->parse(skb, haddr);
}
+/* ll_header must have at least hard_header_len allocated */
+static inline bool dev_validate_header(const struct net_device *dev,
+ char *ll_header, int len)
+{
+ if (likely(len >= dev->hard_header_len))
+ return true;
+
+ if (capable(CAP_SYS_RAWIO)) {
+ memset(ll_header + len, 0, dev->hard_header_len - len);
+ return true;
+ }
+
+ if (dev->header_ops && dev->header_ops->validate)
+ return dev->header_ops->validate(ll_header, len);
+
+ return false;
+}
+
typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr, int len);
int register_gifconf(unsigned int family, gifconf_func_t *gifconf);
static inline int unregister_gifconf(unsigned int family)
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PATCH net-next 2/3] ax25: add link layer header validation function
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
As variable length protocol, AX25 fails link layer header validation
tests based on a minimum length. header_ops.validate allows protocols
to validate headers that are shorter than hard_header_len. Implement
this callback for AX25.
See also http://comments.gmane.org/gmane.linux.network/401064
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ax25/ax25_ip.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/ax25/ax25_ip.c b/net/ax25/ax25_ip.c
index b563a3f..2fa3be9 100644
--- a/net/ax25/ax25_ip.c
+++ b/net/ax25/ax25_ip.c
@@ -228,8 +228,23 @@ netdev_tx_t ax25_ip_xmit(struct sk_buff *skb)
}
#endif
+static bool ax25_validate_header(const char *header, unsigned int len)
+{
+ ax25_digi digi;
+
+ if (!len)
+ return false;
+
+ if (header[0])
+ return true;
+
+ return ax25_addr_parse(header + 1, len - 1, NULL, NULL, &digi, NULL,
+ NULL);
+}
+
const struct header_ops ax25_header_ops = {
.create = ax25_hard_header,
+ .validate = ax25_validate_header,
};
EXPORT_SYMBOL(ax25_header_ops);
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PATCH net-next 3/3] packet: validate variable length ll headers
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
Replace link layer header validation check ll_header_truncate with
more generic dev_validate_header.
Validation based on hard_header_len incorrectly drops valid packets
in variable length protocols, such as AX25. dev_validate_header
calls header_ops.validate for such protocols to ensure correctness
below hard_header_len.
See also http://comments.gmane.org/gmane.linux.network/401064
Fixes 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/packet/af_packet.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d41b107..1ecfa71 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1915,6 +1915,10 @@ retry:
goto retry;
}
+ if (!dev_validate_header(dev, skb->data, len)) {
+ err = -EINVAL;
+ goto out_unlock;
+ }
if (len > (dev->mtu + dev->hard_header_len + extra_len) &&
!packet_extra_vlan_len_allowed(dev, skb)) {
err = -EMSGSIZE;
@@ -2393,18 +2397,6 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
sock_wfree(skb);
}
-static bool ll_header_truncated(const struct net_device *dev, int len)
-{
- /* net device doesn't like empty head */
- if (unlikely(len < dev->hard_header_len)) {
- net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
- current->comm, len, dev->hard_header_len);
- return true;
- }
-
- return false;
-}
-
static void tpacket_set_protocol(const struct net_device *dev,
struct sk_buff *skb)
{
@@ -2522,16 +2514,20 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
if (unlikely(err < 0))
return -EINVAL;
} else if (copylen) {
+ int hdrlen = min_t(int, copylen, tp_len);
+
skb_push(skb, dev->hard_header_len);
skb_put(skb, copylen - dev->hard_header_len);
- err = skb_store_bits(skb, 0, data, copylen);
+ err = skb_store_bits(skb, 0, data, hdrlen);
if (unlikely(err))
return err;
+ if (!dev_validate_header(dev, skb->data, hdrlen))
+ return -EINVAL;
if (!skb->protocol)
tpacket_set_protocol(dev, skb);
- data += copylen;
- to_write -= copylen;
+ data += hdrlen;
+ to_write -= hdrlen;
}
offset = offset_in_page(data);
@@ -2703,13 +2699,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
copylen = __virtio16_to_cpu(vio_le(),
vnet_hdr->hdr_len);
}
- if (dev->hard_header_len) {
- if (ll_header_truncated(dev, tp_len)) {
- tp_len = -EINVAL;
- goto tpacket_error;
- }
- copylen = max_t(int, copylen, dev->hard_header_len);
- }
+ copylen = max_t(int, copylen, dev->hard_header_len);
skb = sock_alloc_send_skb(&po->sk,
hlen + tlen + sizeof(struct sockaddr_ll) +
(copylen - dev->hard_header_len),
@@ -2905,9 +2895,6 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
if (unlikely(offset < 0))
goto out_free;
- } else {
- if (ll_header_truncated(dev, len))
- goto out_free;
}
/* Returns -EFAULT on error */
@@ -2915,6 +2902,12 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
if (err)
goto out_free;
+ if (sock->type == SOCK_RAW &&
+ !dev_validate_header(dev, skb->data, len)) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
if (!vnet_hdr.gso_type && (len > dev->mtu + reserve + extra_len) &&
--
2.7.0.rc3.207.g0ac5344
^ 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