* Re: [PATCH iproute2 1/5] iplink: Fix setting of -1 as ifindex
From: Cong Wang @ 2014-10-07 0:42 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Stephen Hemminger, netdev
In-Reply-To: <1412351718-22921-2-git-send-email-therbert@google.com>
On Fri, Oct 3, 2014 at 8:55 AM, Tom Herbert <therbert@google.com> wrote:
> Commit 3c682146aeff157ec3540 ("iplink: forbid negative ifindex and
> modifying ifindex") initializes index to -1 in iplink_modify. When
> creating a link, req.i.ifi_index is then set to -1 if the link option is
> not used. In the kernel this is then used to set dev->ifindex. For
> dev->ifindex, zero is considered to be unset and -1 is treated as
> a set index, so when a second tunnel is create the new device conflicts
> with the old one (both have ifindex of -1) so -EBUSY is returned.
>
> This patch set zero in req.i.ifi_index is index is unset (still -1).
There was a patch before yours:
https://patchwork.ozlabs.org/patch/395404/
^ permalink raw reply
* [Patch net] net_sched: copy exts->type in tcf_exts_change()
From: Cong Wang @ 2014-10-07 0:21 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, John Fastabend
We need to copy exts->type when committing the change, otherwise
it would be always 0. This is a quick fix for -net and -stable,
for net-next tcf_exts will be removed.
Fixes: commit 33be627159913b094bb578e83 ("net_sched: act: use standard struct list_head")
Reported-by: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c28b0d3..4f4e08b 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -549,6 +549,7 @@ void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst,
tcf_tree_lock(tp);
list_splice_init(&dst->actions, &tmp);
list_splice(&src->actions, &dst->actions);
+ dst->type = src->type;
tcf_tree_unlock(tp);
tcf_action_destroy(&tmp, TCA_ACT_UNBIND);
#endif
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-10-07 0:17 UTC (permalink / raw)
To: Jesse Gross
Cc: Or Gerlitz, Alexander Duyck, John Fastabend, Jeff Kirsher,
David Miller, Linux Netdev List, Thomas Graf, Pravin Shelar,
Andy Zhou
In-Reply-To: <CAEP_g=-S2b3mev+q-qPBHeBw7jb-Y2XvbTeZPgaLgVj=c+3Bjg@mail.gmail.com>
On Mon, Oct 6, 2014 at 3:33 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Oct 6, 2014 at 10:59 AM, Tom Herbert <therbert@google.com> wrote:
>> On Sun, Oct 5, 2014 at 12:13 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Sun, Oct 5, 2014 at 9:49 PM, Tom Herbert <therbert@google.com> wrote:
>>>> On Sun, Oct 5, 2014 at 7:04 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>>> On Thu, Oct 2, 2014 at 2:06 AM, Tom Herbert <therbert@google.com> wrote:
>>>>>> On Wed, Oct 1, 2014 at 1:58 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>>>>> On Tue, Sep 30, 2014 at 6:34 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>> On Tue, Sep 30, 2014 at 7:30 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>>> [...]
>>>>>> Solution #4: apply this patch and implement the check functions as
>>>>>> needed in those 4 or 5 drivers. If a device can only do VXLAN/NVGRE
>>>>>> then I believe the check function is something like:
>>>>>>
>>>>>> bool mydev_gso_check(struct sk_buff *skb, struct net_device *dev)
>>>>>> {
>>>>>> if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
>>>>>> ((skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
>>>>>> skb->protocol != htons(ETH_P_TEB) ||
>>>>>> skb_inner_mac_header(skb) - skb_transport_header(skb) != 12)
>>>>>> return false;
>>>>>>
>>>>>> return true;
>>>>>> }
>>>>>
>>>>> Yep, such helper can can be basically made to work and let the 4-5
>>>>> drivers that can
>>>>> do GSO offloading for vxlan but not for any FOU/GUE packets signal
>>>>> that to the stack.
>>>>>
>>>>> Re the 12 constant, you were referring to the udp+vxlan headers? it's 8+8
>>>>>
>>>>> Also, we need a way for drivers that can support VXLAN or NVGRE but
>>>>> not concurrently
>>>>> on the same port @ the same time to only let vxlan packet to pass
>>>>> successfully through the helper.
>>>
>>>> Or, there should be no difference in GSO processing between VXLAN and
>>>> NVGRE. Can you explain why you feel you need to differentiate them for GSO?
>>>
>>>
>>> RX wise, Linux tells the driver that UDP port X would be used for
>>> VXLAN, right? and indeed, it's possible for some HW implementations
>>> not to support RX offloading (checksum) for both VXLAN and NVGRE @ the
>>> same time over the same port. But TX/GRO wise, you're probably
>>> correct. The thing is that from the user POV they need solution that
>>> works for both RX and TX offloading.
>>
>> I think from a user POV we want a solution that supports RX and TX
>> offloading across the widest range of protocols. This is accomplished
>> by implementing protocol agnostic mechanisms like CHECKSUM_COMPLETE
>> and protocol agnostic UDP tunnel TSO like we've described. IMO, the
>> fact that we have devices that implement protocol specific mechanisms
>> for NVGRE and VXLAN should be considered legacy support in the stack,
>> for new UDP encapsulation protocols we should not expose specifics in
>> the stack in either by adding a GSO type for each protocol, nor
>> ndo_add_foo_port for each protocol-- these things will not scale and
>> unnecessarily complicate the core stack.
>
> It's not clear to me that allowing devices to know what protocols are
> running on what ports actually complicates the stack. The part that is
> complicated is usually the types of operations that are being
> offloaded (checksum, TSO, etc.). In all of these tunnel cases, the
> operations are same and if you have a clean registration mechanism
> then nothing in the core has to see this - only the protocol doing the
> registering and the driver that is supporting it.
>
We already have an ntuple filtering interface that allows configuring
a device for special processing of RX packets. I don't see why that
shouldn't apply to the use case protocol processing for specific ports
in the encapsulation use case.
> I have no disagreement with trying to be generic across protocols. I'm
> just not convinced that it is a realistic plan. It's obvious that it
> is not doable today nor will be it be in the next generation of NICs
> (which are guaranteed to add support for new protocols). Furthermore,
> there will be more advanced stuff coming in the future that I think
> will be difficult or impossible to make protocol agnostic. Rather than
> pretending that this doesn't exist or will never happen, it's better
> focus on how to integrating it cleanly.
Sorry, but I don't understand how supporting a new protocols in a
device for the purposes of returning CHECKSUM_UNNECESSARY is better or
easier to implement than just returning CHECKSUM_COMPLETE. Same thing
for trying to use NETIF_F_IP_CSUM with encapsulation rather than
NETIF_F_HW_CSUM. I'm not a hardware guy, so it's possible I'm missing
something obvious...
Can you be more specific about this "advanced stuff"?
Thanks,
Tom
^ permalink raw reply
* Re: [PATCH RFC v3 net 0/2] ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit
From: Hannes Frederic Sowa @ 2014-10-07 0:10 UTC (permalink / raw)
To: Martin KaFai Lau, netdev
In-Reply-To: <1412640315-22472-1-git-send-email-kafai@fb.com>
On Tue, Oct 7, 2014, at 02:05, Martin KaFai Lau wrote:
> Notes: Same as the last two versions but fixed a title and missing
> Signed-Off issue.
>
> I am trying to understand why there is a need to restart fib6_lookup()
> after
> getting rt with RTF_CACHE.
>
> I have adapted davem's udpflood test
> (https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/pub/scm/linux/kernel/git/davem/net_test_tools.git&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=%2Faj1ZOQObwbmtLwlDw3XzQ%3D%3D%0A&m=j4KoKiV%2FLl4Dx6wOKiDLZPDODlbMJ5UBybTiTzIRHTM%3D%0A&s=68cac2d1d239e23b104065419b4ad89ea80bc1401571034ccce3b4a52a98a8d3)
> to
> support IPv6 and here is the result:
>
> #root > time ./udpflood -l 20000000 -c 250 2401:db00:face:face::2
>
> Before:
> real 0m33.224s
> user 0m2.941s
> sys 0m30.232s
>
> After:
> real 0m31.517s
> user 0m2.938s
> sys 0m28.536s
Thanks, Martin! I'll review them ASAP.
Bye,
Hannes
^ permalink raw reply
* [PATCH RFC v3 net 0/2] ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit
From: Martin KaFai Lau @ 2014-10-07 0:05 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
Notes: Same as the last two versions but fixed a title and missing
Signed-Off issue.
I am trying to understand why there is a need to restart fib6_lookup() after
getting rt with RTF_CACHE.
I have adapted davem's udpflood test
(https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/pub/scm/linux/kernel/git/davem/net_test_tools.git&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=%2Faj1ZOQObwbmtLwlDw3XzQ%3D%3D%0A&m=j4KoKiV%2FLl4Dx6wOKiDLZPDODlbMJ5UBybTiTzIRHTM%3D%0A&s=68cac2d1d239e23b104065419b4ad89ea80bc1401571034ccce3b4a52a98a8d3) to
support IPv6 and here is the result:
#root > time ./udpflood -l 20000000 -c 250 2401:db00:face:face::2
Before:
real 0m33.224s
user 0m2.941s
sys 0m30.232s
After:
real 0m31.517s
user 0m2.938s
sys 0m28.536s
/****************************** udpflood.c ******************************/
/* It is an adaptation of the Eric Dumazet's and David Miller's
* udpflood tool, by adding IPv6 support.
*/
#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>
#include <malloc.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <stdint.h>
#include <assert.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#define _GNU_SOURCE
#include <getopt.h>
typedef uint32_t u32;
static int debug = 0;
/* Allow -fstrict-aliasing */
typedef union sa_u {
struct sockaddr_storage a46;
struct sockaddr_in a4;
struct sockaddr_in6 a6;
} sa_u;
static int usage(void)
{
printf("usage: udpflood [ -l count ] [ -m message_size ] [ -c num_ip_addrs ] IP_ADDRESS\n");
return -1;
}
static u32 get_last32h(const sa_u *sa)
{
if (sa->a46.ss_family == PF_INET)
return ntohl(sa->a4.sin_addr.s_addr);
else
return ntohl(sa->a6.sin6_addr.s6_addr32[3]);
}
static void set_last32h(sa_u *sa, u32 last32h)
{
if (sa->a46.ss_family == PF_INET)
sa->a4.sin_addr.s_addr = htonl(last32h);
else
sa->a6.sin6_addr.s6_addr32[3] = htonl(last32h);
}
static void print_saddr(const sa_u *sa, const char *msg)
{
char buf[64];
if (!debug)
return;
switch (sa->a46.ss_family) {
case PF_INET:
inet_ntop(PF_INET, &(sa->a4.sin_addr.s_addr), buf,
sizeof(buf));
break;
case PF_INET6:
inet_ntop(PF_INET6, &(sa->a6.sin6_addr), buf, sizeof(buf));
break;
}
printf("%s: %s\n", msg, buf);
}
static int send_packets(const sa_u *sa, size_t num_addrs, int count, int msg_sz)
{
char *msg = malloc(msg_sz);
sa_u saddr;
u32 start_addr32h, end_addr32h, cur_addr32h;
int fd, i, err;
if (!msg)
return -ENOMEM;
memset(msg, 0, msg_sz);
memcpy(&saddr, sa, sizeof(saddr));
cur_addr32h = start_addr32h = get_last32h(&saddr);
end_addr32h = start_addr32h + num_addrs;
fd = socket(saddr.a46.ss_family, SOCK_DGRAM, 0);
if (fd < 0) {
perror("socket");
err = fd;
goto out_nofd;
}
/* connect to avoid the kernel spending time in figuring
* out the source address (i.e pin the src address)
*/
err = connect(fd, (struct sockaddr *) &saddr, sizeof(saddr));
if (err < 0) {
perror("connect");
goto out;
}
print_saddr(&saddr, "start_addr");
for (i = 0; i < count; i++) {
print_saddr(&saddr, "sendto");
err = sendto(fd, msg, msg_sz, 0, (struct sockaddr *)&saddr,
sizeof(saddr));
if (err < 0) {
perror("sendto");
goto out;
}
if (++cur_addr32h >= end_addr32h)
cur_addr32h = start_addr32h;
set_last32h(&saddr, cur_addr32h);
}
err = 0;
out:
close(fd);
out_nofd:
free(msg);
return err;
}
int main(int argc, char **argv, char **envp)
{
int port, msg_sz, count, num_addrs, ret;
sa_u start_addr;
port = 6000;
msg_sz = 32;
count = 10000000;
num_addrs = 1;
while ((ret = getopt(argc, argv, "dl:s:p:c:")) >= 0) {
switch (ret) {
case 'l':
sscanf(optarg, "%d", &count);
break;
case 's':
sscanf(optarg, "%d", &msg_sz);
break;
case 'p':
sscanf(optarg, "%d", &port);
break;
case 'c':
sscanf(optarg, "%d", &num_addrs);
break;
case 'd':
debug = 1;
break;
case '?':
return usage();
}
}
if (num_addrs < 1)
return usage();
if (!argv[optind])
return usage();
start_addr.a4.sin_port = htons(port);
if (inet_pton(PF_INET, argv[optind], &start_addr.a4.sin_addr))
start_addr.a46.ss_family = PF_INET;
else if (inet_pton(PF_INET6, argv[optind], &start_addr.a6.sin6_addr.s6_addr))
start_addr.a46.ss_family = PF_INET6;
else
return usage();
return send_packets(&start_addr, num_addrs, count, msg_sz);
}
^ permalink raw reply
* [PATCH RFC v3 net 1/2] ipv6: Remove the net->ipv6.ip6_null_entry check
From: Martin KaFai Lau @ 2014-10-07 0:05 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <1412640315-22472-1-git-send-email-kafai@fb.com>
The above BACKTRACK have already caught the rt == net->ipv6.ip6_null_entry case
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bafde82..d53dc4f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -936,8 +936,7 @@ restart:
if (rt->rt6i_nsiblings)
rt = rt6_multipath_select(rt, fl6, oif, strict | reachable);
BACKTRACK(net, &fl6->saddr);
- if (rt == net->ipv6.ip6_null_entry ||
- rt->rt6i_flags & RTF_CACHE)
+ if (rt->rt6i_flags & RTF_CACHE)
goto out;
dst_hold(&rt->dst);
--
1.8.1
^ permalink raw reply related
* [PATCH RFC v3 net 2/2] ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit case
From: Martin KaFai Lau @ 2014-10-07 0:05 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <1412640315-22472-1-git-send-email-kafai@fb.com>
When there is a RTF_CACHE hit, no need to redo fib6_lookup()
with reachable=0.
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d53dc4f..e40b5dc 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -937,7 +937,7 @@ restart:
rt = rt6_multipath_select(rt, fl6, oif, strict | reachable);
BACKTRACK(net, &fl6->saddr);
if (rt->rt6i_flags & RTF_CACHE)
- goto out;
+ goto out1;
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
@@ -974,6 +974,7 @@ out:
reachable = 0;
goto restart_2;
}
+out1:
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
out2:
--
1.8.1
^ permalink raw reply related
* [Patch net-next] net_sched: fix unused variables in __gnet_stats_copy_basic_cpu()
From: Cong Wang @ 2014-10-07 0:01 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, John Fastabend
Probably not a big deal, but we'd better just use the
one we get in retry loop.
Fixes: commit 22e0f8b9322cb1a48b1357e8 ("net: sched: make bstats per cpu and estimator RCU safe")
Reported-by: Joe Perches <joe@perches.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/core/gen_stats.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 14681b9..0c08062 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -106,8 +106,8 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
for_each_possible_cpu(i) {
struct gnet_stats_basic_cpu *bcpu = per_cpu_ptr(cpu, i);
unsigned int start;
- __u64 bytes;
- __u32 packets;
+ u64 bytes;
+ u32 packets;
do {
start = u64_stats_fetch_begin_irq(&bcpu->syncp);
@@ -115,8 +115,8 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
packets = bcpu->bstats.packets;
} while (u64_stats_fetch_retry_irq(&bcpu->syncp, start));
- bstats->bytes += bcpu->bstats.bytes;
- bstats->packets += bcpu->bstats.packets;
+ bstats->bytes += bytes;
+ bstats->packets += packets;
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] icmp6: Add new icmpv6 type for RPL control message
From: Hannes Frederic Sowa @ 2014-10-06 23:53 UTC (permalink / raw)
To: David Miller, simon.vincent; +Cc: netdev
In-Reply-To: <20141006.181322.2253854250567400185.davem@davemloft.net>
On Tue, Oct 7, 2014, at 00:13, David Miller wrote:
> From: Simon Vincent <simon.vincent@xsilon.com>
> Date: Mon, 6 Oct 2014 11:37:06 +0100
>
> > IANA has defined a type value of 155 for RPL control messages.
> > We do nothing if we recieve one of these messages. This patch is to
> > avoid getting lots of icmpv6 unknown type messages when using RPL.
> >
> > Signed-off-by: Simon Vincent <simon.vincent@xsilon.com>
>
> If we agree that pretty much our policy is that we treat as "known"
> any ICMPv6 type assigned officially by IANA, then we should simply
> add everything missing from the table at:
>
> http://www.iana.org/assignments/icmpv6-parameters/icmpv6-parameters.xhtml
>
> Any objections?
Might be possible, but I would favor to get rid of the printk or move
the test for informational icmp notifications up.
Some of the type < 128 icmp (non-informal) packets we also report to
user space, so we cannot just add them to a blacklist.
Bye,
Hannes
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Hannes Frederic Sowa @ 2014-10-06 23:26 UTC (permalink / raw)
To: John Fastabend
Cc: Daniel Borkmann, John Fastabend, Jesper Dangaard Brouer,
John W. Linville, Neil Horman, Florian Westphal, gerlitz.or,
netdev, john.ronciak, amirv, eric.dumazet, danny.zhou,
Willem de Bruijn
In-Reply-To: <5432FD6D.2020102@intel.com>
Hi John,
On Mon, Oct 6, 2014, at 22:37, John Fastabend wrote:
> > I find the six additional ndo ops a bit worrisome as we are adding more
> > and more subsystem specific ndoops to this struct. I would like to see
> > some unification here, but currently cannot make concrete proposals,
> > sorry.
>
> I agree it seems like a bit much. One thought was to split the ndo
> ops into categories. Switch ops, MACVLAN ops, basic ops and with this
> userspace queue ops. This sort of goes along with some of the switch
> offload work which is going to add a handful more ops as best I can
> tell.
Thanks for your mail, you answered all of my questions.
Have you looked at <https://code.google.com/p/kernel/wiki/ProjectUnetq>?
Willem (also in Cc) used sysfs files which get mmaped to represent the
tx/rx descriptors. The representation was independent of the device and
IIRC the prototype used a write(fd, "", 1) to signal the kernel it
should proceed with tx. I agree, it would be great to be syscall-free
here.
For the semantics of the descriptors we could also easily generate files
in sysfs. I thought about something like tracepoints already do for
representing the data in the ringbuffer depending on the event:
-- >8 --
# cat /sys/kernel/debug/tracing/events/net/net_dev_queue/format
name: net_dev_queue
ID: 1006
format:
field:unsigned short common_type; offset:0; size:2;
signed:0;
field:unsigned char common_flags; offset:2; size:1;
signed:0;
field:unsigned char common_preempt_count; offset:3;
size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:void * skbaddr; offset:8; size:8; signed:0;
field:unsigned int len; offset:16; size:4; signed:0;
field:__data_loc char[] name; offset:20; size:4;
signed:1;
print fmt: "dev=%s skbaddr=%p len=%u", __get_str(name), REC->skbaddr,
REC->len
-- >8 --
Maybe the macros from tracing are reusable (TP_STRUCT__entry), e.g.
endianess would need to be added. Hopefully there is already a user
space parser somewhere in the perf sources. An easier to parse binary
representation could be added easily and maybe even something vDSO alike
if people care about that.
Maybe this open/mmap per queue also kills some of the ndo_ops?
Bye,
Hannes
^ permalink raw reply
* [PATCH ethtool v2 2/3] ethtool: Add copybreak support
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan
In-Reply-To: <1412637141-3205-1-git-send-email-_govind@gmx.com>
This patch adds support for setting/getting driver's rx_copybreak value.
copybreak is set/get using new ethtool tunable interface.
This was added to net-next in
commit: f0db9b073415848709dd59a6394969882f517da9
ethtool: Add generic options for tunables
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
ethtool.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/ethtool.c b/ethtool.c
index bf583f3..4045356 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -179,6 +179,12 @@ static const struct flag_info flags_msglvl[] = {
{ "wol", NETIF_MSG_WOL },
};
+static const char *tunable_name[] = {
+ [ETHTOOL_ID_UNSPEC] = "Unspec",
+ [ETHTOOL_RX_COPYBREAK] = "rx",
+ [ETHTOOL_TX_COPYBREAK] = "tx",
+};
+
struct off_flag_def {
const char *short_name;
const char *long_name;
@@ -1805,6 +1811,173 @@ static int do_gring(struct cmd_context *ctx)
return 0;
}
+static int get_u32tunable(struct cmd_context *ctx, enum tunable_id id,
+ __u32 *value)
+{
+ struct ethtool_tunable *etuna;
+ int ret;
+
+ etuna = calloc(sizeof(*etuna) + sizeof(__u32), 1);
+ if (!etuna)
+ return 1;
+ etuna->cmd = ETHTOOL_GTUNABLE;
+ etuna->id = id;
+ etuna->type_id = ETHTOOL_TUNABLE_U32;
+ etuna->len = sizeof(__u32);
+ ret = send_ioctl(ctx, etuna);
+ *value = *(__u32 *)((void *)etuna + sizeof(*etuna));
+ free(etuna);
+
+ return ret;
+}
+
+static int print_u32tunable(int err, enum tunable_id id, const __u32 value)
+{
+ if (err) {
+ switch (errno) {
+ /* Driver does not support this particular tunable
+ * Usually displays 0
+ */
+ case EINVAL:
+ goto print;
+ /* Driver does not support get tunables ops or no such device
+ * No point in proceeding further
+ */
+ case EOPNOTSUPP:
+ case ENODEV:
+ perror("Cannot get device settings");
+ exit(err);
+ default:
+ perror(tunable_name[id]);
+ return err;
+ }
+ }
+print:
+ fprintf(stdout, "%s: %u\n", tunable_name[id], value);
+
+ return 0;
+}
+
+static int do_gcopybreak(struct cmd_context *ctx)
+{
+ int err, anyerror = 0;
+ __u32 u32value;
+
+ if (ctx->argc != 0)
+ exit_bad_args();
+
+ fprintf(stdout, "Copybreak settings for device %s\n", ctx->devname);
+
+ err = get_u32tunable(ctx, ETHTOOL_RX_COPYBREAK, &u32value);
+ err = print_u32tunable(err, ETHTOOL_RX_COPYBREAK, u32value);
+ if (err)
+ anyerror = err;
+
+ err = get_u32tunable(ctx, ETHTOOL_TX_COPYBREAK, &u32value);
+ err = print_u32tunable(err, ETHTOOL_TX_COPYBREAK, u32value);
+ if (err)
+ anyerror = err;
+
+ if (anyerror)
+ fprintf(stderr, "Failed to get all settings. displayed partial settings\n");
+
+ return anyerror;
+}
+
+static int set_u32tunable(struct cmd_context *ctx, enum tunable_id id,
+ const __u32 value)
+{
+ struct ethtool_tunable *etuna;
+ int ret;
+ __u32 *data;
+
+ etuna = malloc(sizeof(*etuna) + sizeof(__u32));
+ if (!etuna) {
+ perror(tunable_name[id]);
+ return 1;
+ }
+ data = (void *)etuna + sizeof(*etuna);
+ *data = value;
+ etuna->cmd = ETHTOOL_STUNABLE;
+ etuna->id = id;
+ etuna->type_id = ETHTOOL_TUNABLE_U32;
+ etuna->len = sizeof(__u32);
+ ret = send_ioctl(ctx, etuna);
+ free(etuna);
+
+ return ret;
+}
+
+static int check_set_u32tunable(int err, enum tunable_id id)
+{
+ if (err) {
+ switch (errno) {
+ /* Driver does not support get tunables ops or no such device
+ * No point in proceeding further
+ */
+ case EOPNOTSUPP:
+ case ENODEV:
+ perror("Cannot set device settings");
+ exit(err);
+ default:
+ perror(tunable_name[id]);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+static int do_scopybreak(struct cmd_context *ctx)
+{
+ int err, anyerr = 0;
+ int copybreak_changed = 0;
+ __u32 rx, tx;
+ s32 rx_seen = 0;
+ s32 tx_seen = 0;
+
+ struct cmdline_info cmdline_channels[] = {
+ { .name = "rx",
+ .type = CMDL_U32,
+ .wanted_val = &rx,
+ .seen_val = &rx_seen, },
+
+ { .name = "tx",
+ .type = CMDL_U32,
+ .wanted_val = &tx,
+ .seen_val = &tx_seen, },
+ };
+
+ parse_generic_cmdline(ctx, ©break_changed, cmdline_channels,
+ ARRAY_SIZE(cmdline_channels));
+
+ if (!copybreak_changed) {
+ fprintf(stderr, "no copybreak parameters changed\n");
+ return 0;
+ }
+
+ if (rx_seen) {
+ err = set_u32tunable(ctx, ETHTOOL_RX_COPYBREAK, rx);
+ err = check_set_u32tunable(err, ETHTOOL_RX_COPYBREAK);
+ if (err)
+ anyerr = err;
+ }
+
+ if (tx_seen) {
+ err = set_u32tunable(ctx, ETHTOOL_TX_COPYBREAK, tx);
+ err = check_set_u32tunable(err, ETHTOOL_TX_COPYBREAK);
+ if (err)
+ anyerr = err;
+ }
+
+ if (anyerr) {
+ fprintf(stderr, "Failed to set all requested parameters\n");
+ return anyerr;
+ }
+
+ return 0;
+}
+
static int do_schannels(struct cmd_context *ctx)
{
struct ethtool_channels echannels;
@@ -4055,6 +4228,10 @@ static const struct option {
" [ rx-mini N ]\n"
" [ rx-jumbo N ]\n"
" [ tx N ]\n" },
+ { "-b|--show-copybreak", 1, do_gcopybreak, "Show copybreak values" },
+ { "-B|--set-copybreak", 1, do_scopybreak, "Set copybreak values",
+ " [ rx N]\n"
+ " [ tx N]\n" },
{ "-k|--show-features|--show-offload", 1, do_gfeatures,
"Get state of protocol offload and other features" },
{ "-K|--features|--offload", 1, do_sfeatures,
--
2.1.0
^ permalink raw reply related
* [PATCH ethtool v2 3/3] ethtool.8.in: Add man page for copybreak
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan
In-Reply-To: <1412637141-3205-1-git-send-email-_govind@gmx.com>
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
ethtool.8.in | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/ethtool.8.in b/ethtool.8.in
index ae56293..9f0955f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -176,6 +176,14 @@ ethtool \- query or control network driver and hardware settings
.BN rx\-jumbo
.BN tx
.HP
+.B ethtool \-b|\-\-show\-copybreak
+.I devname
+.HP
+.B ethtool \-B|\-\-set\-copybreak
+.I devname
+.BN rx
+.BN tx
+.HP
.B ethtool \-i|\-\-driver
.I devname
.HP
@@ -399,6 +407,18 @@ Changes the number of ring entries for the Rx Jumbo ring.
.BI tx \ N
Changes the number of ring entries for the Tx ring.
.TP
+.B \-b|\-\-show\-copybreak
+Get device copybreak values
+.TP
+.B \-B|\-\-set\-copybreak
+Set device copybreak values
+.TP
+.BI rx \ N
+Change rx_copybreak
+.TP
+.BI tx \ N
+Change tx_copybreak
+.TP
.B \-i \-\-driver
Queries the specified network device for associated driver information.
.TP
--
2.1.0
^ permalink raw reply related
* [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan
In-Reply-To: <1412637141-3205-1-git-send-email-_govind@gmx.com>
This covers kernel changes up to:
commit: 1255a5055449781a92076fc5429952f2b33cf309
Author: Eric Dumazet <edumazet@google.com>
Date: Sun Oct 5 12:35:21 2014 +0300
ethtool: Ethtool parameter to dynamically change tx_copybreak
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
ethtool-copy.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/ethtool-copy.h b/ethtool-copy.h
index 61b78fc..3c89379 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -209,6 +209,33 @@ struct ethtool_value {
__u32 data;
};
+enum tunable_id {
+ ETHTOOL_ID_UNSPEC,
+ ETHTOOL_RX_COPYBREAK,
+ ETHTOOL_TX_COPYBREAK,
+};
+
+enum tunable_type_id {
+ ETHTOOL_TUNABLE_UNSPEC,
+ ETHTOOL_TUNABLE_U8,
+ ETHTOOL_TUNABLE_U16,
+ ETHTOOL_TUNABLE_U32,
+ ETHTOOL_TUNABLE_U64,
+ ETHTOOL_TUNABLE_STRING,
+ ETHTOOL_TUNABLE_S8,
+ ETHTOOL_TUNABLE_S16,
+ ETHTOOL_TUNABLE_S32,
+ ETHTOOL_TUNABLE_S64,
+};
+
+struct ethtool_tunable {
+ __u32 cmd;
+ __u32 id;
+ __u32 type_id;
+ __u32 len;
+ void *data[0];
+};
+
/**
* struct ethtool_regs - hardware register dump
* @cmd: Command number = %ETHTOOL_GREGS
@@ -1152,6 +1179,8 @@ enum ethtool_sfeatures_retval_bits {
#define ETHTOOL_GRSSH 0x00000046 /* Get RX flow hash configuration */
#define ETHTOOL_SRSSH 0x00000047 /* Set RX flow hash configuration */
+#define ETHTOOL_GTUNABLE 0x00000048 /* Get tunable configuration */
+#define ETHTOOL_STUNABLE 0x00000049 /* Set tunable configuration */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
--
2.1.0
^ permalink raw reply related
* [PATCH ethtool v2 0/3] Add copybreak support
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan
This series add support for setting/getting driver's copybreak value.
copybreak is set/get using ethtool tunable interface.
This was added to net-next in
commit: f0db9b073415848709dd59a6394969882f517da9
ethtool: Add generic options for tunables
v2:
Add support for tx_copybreak
Add 'ethtool -B eth0 rx 100 tx 256' instead of 'ethtool -B eth0 100'
Govindarajulu Varadarajan (3):
ethtool-copy.h: Sync with net-next 3.17.0-rc7
ethtool: Add copybreak support
ethtool.8.in: Add man page for copybreak
ethtool-copy.h | 29 ++++++++++
ethtool.8.in | 20 +++++++
ethtool.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
--
2.1.0
^ permalink raw reply
* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Luis R. Rodriguez @ 2014-10-06 23:10 UTC (permalink / raw)
To: Tejun Heo
Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141006210118.GG18303@htj.dyndns.org>
On Mon, Oct 06, 2014 at 05:01:18PM -0400, Tejun Heo wrote:
> Hello,
>
> On Mon, Oct 06, 2014 at 10:36:27PM +0200, Luis R. Rodriguez wrote:
> > > Do we intend to keep this param permanently? Isn't this more of a
> > > temp tool to be used during development? If so, maybe we should make
> > > that clear with __DEVEL__ too?
> >
> > As its designed right now no, its not a temp tool, its there to
> > require compatibility with old userspace. For modules we can require
> > the module parameter but for built-in we need something else and this
> > is what came to mind. It is also what would allow the prefer_async_probe
> > flag too as otherwise we won't know if userspace is prepared.
>
> I don't get it.
By prepared I meant that userspace can handle async probe, but
you're right that we don't need to know that. I don't see how
we'd be breaking old userspace by doing async probe of a driver
is built-in right now... unless of course built-in always assumes
all possible devices would be present after right before userspace
init.
> For in-kernel stuff, we already have a clear
> synchronization point where we already synchronize all async calls.
> Shouldn't we be flushing these async probes there too?
This seems to be addressing if what I meant by prepared, "ready", so let
me address this as I do think its important.
By async calls do you mean users of async_schedule()? I see it
also uses system_unbound_wq as well but I do not see anyone calling
flush_workqueue(system_unbound_wq) on the kernel. We do use
async_synchronize_full() on kernel_init() but that just waits.
As it is we don't wait on init then, should we? Must we? Could / should
we use bus.enable_kern_async=1 to enable avoiding having to wait ? At
this point I'd prefer to address what we must do only.
> insmod'ing is
> userland visible but there's no reason this has to be for the built-in
> drivers.
Good point.
bus.enable_kern_async=1 would still also serve as a helper for the driver core
to figure out if it should use async probe then on modules if prefer_async_probe
was enabled. Let me know if you figure out a way to avoid it.
Luis
^ permalink raw reply
* [PATCH RFC v2 net 2/2] File Edit Options Buffers Tools Help ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit case
From: Martin KaFai Lau @ 2014-10-06 22:42 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <1412635342-6711-1-git-send-email-kafai@fb.com>
When there is a RTF_CACHE hit, no need to redo fib6_lookup()
with reachable=0.
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d53dc4f..e40b5dc 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -937,7 +937,7 @@ restart:
rt = rt6_multipath_select(rt, fl6, oif, strict | reachable);
BACKTRACK(net, &fl6->saddr);
if (rt->rt6i_flags & RTF_CACHE)
- goto out;
+ goto out1;
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
@@ -974,6 +974,7 @@ out:
reachable = 0;
goto restart_2;
}
+out1:
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
out2:
--
1.8.1
^ permalink raw reply related
* [PATCH RFC v2 net 0/2] ipv6: Avoid restarting fib6_lookup() for RTF_CACHE hit
From: Martin KaFai Lau @ 2014-10-06 22:42 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
I am trying to understand why there is a need to restart fib6_lookup() after
getting rt with RTF_CACHE.
I have adapted davem's udpflood test
(https://urldefense.proofpoint.com/v1/url?u=https://git.kernel.org/pub/scm/linux/kernel/git/davem/net_test_tools.git&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=%2Faj1ZOQObwbmtLwlDw3XzQ%3D%3D%0A&m=j4KoKiV%2FLl4Dx6wOKiDLZPDODlbMJ5UBybTiTzIRHTM%3D%0A&s=68cac2d1d239e23b104065419b4ad89ea80bc1401571034ccce3b4a52a98a8d3) to
support IPv6 and here is the result:
#root > time ./udpflood -l 20000000 -c 250 2401:db00:face:face::2
Before:
real 0m33.224s
user 0m2.941s
sys 0m30.232s
After:
real 0m31.517s
user 0m2.938s
sys 0m28.536s
/****************************** udpflood.c ******************************/
/* It is an adaptation of the Eric Dumazet's and David Miller's
* udpflood tool, by adding IPv6 support.
*/
#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>
#include <malloc.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <stdint.h>
#include <assert.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#define _GNU_SOURCE
#include <getopt.h>
typedef uint32_t u32;
static int debug = 0;
/* Allow -fstrict-aliasing */
typedef union sa_u {
struct sockaddr_storage a46;
struct sockaddr_in a4;
struct sockaddr_in6 a6;
} sa_u;
static int usage(void)
{
printf("usage: udpflood [ -l count ] [ -m message_size ] [ -c num_ip_addrs ] IP_ADDRESS\n");
return -1;
}
static u32 get_last32h(const sa_u *sa)
{
if (sa->a46.ss_family == PF_INET)
return ntohl(sa->a4.sin_addr.s_addr);
else
return ntohl(sa->a6.sin6_addr.s6_addr32[3]);
}
static void set_last32h(sa_u *sa, u32 last32h)
{
if (sa->a46.ss_family == PF_INET)
sa->a4.sin_addr.s_addr = htonl(last32h);
else
sa->a6.sin6_addr.s6_addr32[3] = htonl(last32h);
}
static void print_saddr(const sa_u *sa, const char *msg)
{
char buf[64];
if (!debug)
return;
switch (sa->a46.ss_family) {
case PF_INET:
inet_ntop(PF_INET, &(sa->a4.sin_addr.s_addr), buf,
sizeof(buf));
break;
case PF_INET6:
inet_ntop(PF_INET6, &(sa->a6.sin6_addr), buf, sizeof(buf));
break;
}
printf("%s: %s\n", msg, buf);
}
static int send_packets(const sa_u *sa, size_t num_addrs, int count, int msg_sz)
{
char *msg = malloc(msg_sz);
sa_u saddr;
u32 start_addr32h, end_addr32h, cur_addr32h;
int fd, i, err;
if (!msg)
return -ENOMEM;
memset(msg, 0, msg_sz);
memcpy(&saddr, sa, sizeof(saddr));
cur_addr32h = start_addr32h = get_last32h(&saddr);
end_addr32h = start_addr32h + num_addrs;
fd = socket(saddr.a46.ss_family, SOCK_DGRAM, 0);
if (fd < 0) {
perror("socket");
err = fd;
goto out_nofd;
}
/* connect to avoid the kernel spending time in figuring
* out the source address (i.e pin the src address)
*/
err = connect(fd, (struct sockaddr *) &saddr, sizeof(saddr));
if (err < 0) {
perror("connect");
goto out;
}
print_saddr(&saddr, "start_addr");
for (i = 0; i < count; i++) {
print_saddr(&saddr, "sendto");
err = sendto(fd, msg, msg_sz, 0, (struct sockaddr *)&saddr,
sizeof(saddr));
if (err < 0) {
perror("sendto");
goto out;
}
if (++cur_addr32h >= end_addr32h)
cur_addr32h = start_addr32h;
set_last32h(&saddr, cur_addr32h);
}
err = 0;
out:
close(fd);
out_nofd:
free(msg);
return err;
}
int main(int argc, char **argv, char **envp)
{
int port, msg_sz, count, num_addrs, ret;
sa_u start_addr;
port = 6000;
msg_sz = 32;
count = 10000000;
num_addrs = 1;
while ((ret = getopt(argc, argv, "dl:s:p:c:")) >= 0) {
switch (ret) {
case 'l':
sscanf(optarg, "%d", &count);
break;
case 's':
sscanf(optarg, "%d", &msg_sz);
break;
case 'p':
sscanf(optarg, "%d", &port);
break;
case 'c':
sscanf(optarg, "%d", &num_addrs);
break;
case 'd':
debug = 1;
break;
case '?':
return usage();
}
}
if (num_addrs < 1)
return usage();
if (!argv[optind])
return usage();
start_addr.a4.sin_port = htons(port);
if (inet_pton(PF_INET, argv[optind], &start_addr.a4.sin_addr))
start_addr.a46.ss_family = PF_INET;
else if (inet_pton(PF_INET6, argv[optind], &start_addr.a6.sin6_addr.s6_addr))
start_addr.a46.ss_family = PF_INET6;
else
return usage();
return send_packets(&start_addr, num_addrs, count, msg_sz);
}
^ permalink raw reply
* [PATCH RFC v2 net 1/2] ipv6: Remove the net->ipv6.ip6_null_entry check
From: Martin KaFai Lau @ 2014-10-06 22:42 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <1412635342-6711-1-git-send-email-kafai@fb.com>
The above BACKTRACK have already caught the rt == net->ipv6.ip6_null_entry case
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bafde82..d53dc4f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -936,8 +936,7 @@ restart:
if (rt->rt6i_nsiblings)
rt = rt6_multipath_select(rt, fl6, oif, strict | reachable);
BACKTRACK(net, &fl6->saddr);
- if (rt == net->ipv6.ip6_null_entry ||
- rt->rt6i_flags & RTF_CACHE)
+ if (rt->rt6i_flags & RTF_CACHE)
goto out;
dst_hold(&rt->dst);
--
1.8.1
^ permalink raw reply related
* Re: [PATCH] net: Add ndo_gso_check
From: Jesse Gross @ 2014-10-06 22:33 UTC (permalink / raw)
To: Tom Herbert
Cc: Or Gerlitz, Alexander Duyck, John Fastabend, Jeff Kirsher,
David Miller, Linux Netdev List, Thomas Graf, Pravin Shelar,
Andy Zhou
In-Reply-To: <CA+mtBx-AW65w7grfb7zTaDKzKQ2n_7JQuLquj1Ayopa4gdiF6Q@mail.gmail.com>
On Mon, Oct 6, 2014 at 10:59 AM, Tom Herbert <therbert@google.com> wrote:
> On Sun, Oct 5, 2014 at 12:13 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Sun, Oct 5, 2014 at 9:49 PM, Tom Herbert <therbert@google.com> wrote:
>>> On Sun, Oct 5, 2014 at 7:04 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>> On Thu, Oct 2, 2014 at 2:06 AM, Tom Herbert <therbert@google.com> wrote:
>>>>> On Wed, Oct 1, 2014 at 1:58 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>>>> On Tue, Sep 30, 2014 at 6:34 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>> On Tue, Sep 30, 2014 at 7:30 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>> [...]
>>>>> Solution #4: apply this patch and implement the check functions as
>>>>> needed in those 4 or 5 drivers. If a device can only do VXLAN/NVGRE
>>>>> then I believe the check function is something like:
>>>>>
>>>>> bool mydev_gso_check(struct sk_buff *skb, struct net_device *dev)
>>>>> {
>>>>> if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
>>>>> ((skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
>>>>> skb->protocol != htons(ETH_P_TEB) ||
>>>>> skb_inner_mac_header(skb) - skb_transport_header(skb) != 12)
>>>>> return false;
>>>>>
>>>>> return true;
>>>>> }
>>>>
>>>> Yep, such helper can can be basically made to work and let the 4-5
>>>> drivers that can
>>>> do GSO offloading for vxlan but not for any FOU/GUE packets signal
>>>> that to the stack.
>>>>
>>>> Re the 12 constant, you were referring to the udp+vxlan headers? it's 8+8
>>>>
>>>> Also, we need a way for drivers that can support VXLAN or NVGRE but
>>>> not concurrently
>>>> on the same port @ the same time to only let vxlan packet to pass
>>>> successfully through the helper.
>>
>>> Or, there should be no difference in GSO processing between VXLAN and
>>> NVGRE. Can you explain why you feel you need to differentiate them for GSO?
>>
>>
>> RX wise, Linux tells the driver that UDP port X would be used for
>> VXLAN, right? and indeed, it's possible for some HW implementations
>> not to support RX offloading (checksum) for both VXLAN and NVGRE @ the
>> same time over the same port. But TX/GRO wise, you're probably
>> correct. The thing is that from the user POV they need solution that
>> works for both RX and TX offloading.
>
> I think from a user POV we want a solution that supports RX and TX
> offloading across the widest range of protocols. This is accomplished
> by implementing protocol agnostic mechanisms like CHECKSUM_COMPLETE
> and protocol agnostic UDP tunnel TSO like we've described. IMO, the
> fact that we have devices that implement protocol specific mechanisms
> for NVGRE and VXLAN should be considered legacy support in the stack,
> for new UDP encapsulation protocols we should not expose specifics in
> the stack in either by adding a GSO type for each protocol, nor
> ndo_add_foo_port for each protocol-- these things will not scale and
> unnecessarily complicate the core stack.
It's not clear to me that allowing devices to know what protocols are
running on what ports actually complicates the stack. The part that is
complicated is usually the types of operations that are being
offloaded (checksum, TSO, etc.). In all of these tunnel cases, the
operations are same and if you have a clean registration mechanism
then nothing in the core has to see this - only the protocol doing the
registering and the driver that is supporting it.
I have no disagreement with trying to be generic across protocols. I'm
just not convinced that it is a realistic plan. It's obvious that it
is not doable today nor will be it be in the next generation of NICs
(which are guaranteed to add support for new protocols). Furthermore,
there will be more advanced stuff coming in the future that I think
will be difficult or impossible to make protocol agnostic. Rather than
pretending that this doesn't exist or will never happen, it's better
focus on how to integrating it cleanly.
^ permalink raw reply
* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: josh @ 2014-10-06 22:27 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, netdev, linux-kernel, john.fastabend
In-Reply-To: <1412633064.2916.46.camel@joe-AO725>
On Mon, Oct 06, 2014 at 03:04:24PM -0700, Joe Perches wrote:
> On Mon, 2014-10-06 at 17:56 -0400, David Miller wrote:
> > From: Joe Perches <joe@perches.com>
> > Date: Mon, 06 Oct 2014 14:51:38 -0700
> >
> > > Reduce noise when compiling W=1.
> []
> > BTW, this patch reminds me that if people think there are
> > subdirectories where we can turn on things like -Werror in the
> > networking I would be very happy to apply such patches.
> []
> > Things like net/core/ for example should be doable for sure.
>
> I don't have any significant opposition to -Werror, but
> I think there are real arguments _against_ using -Werror.
>
> I think the primary one is new compiler versions have a
> tendency to add new warnings for various things that can
> unnecessarily and unpredictably break the build.
-Werror is a bad idea, even on a per-directory basis. However,
-Werror=specific-warning is a great idea. We should add that for
high-value warnings that have been entirely eliminated in a directory.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH net-next] net: introduce netdevice gso_min_segs attribute
From: Eric Dumazet @ 2014-10-06 22:26 UTC (permalink / raw)
To: David Miller; +Cc: amirv, edumazet, netdev, yevgenyp, ogerlitz, idos
In-Reply-To: <20141006.175410.2083551367207718756.davem@davemloft.net>
On Mon, 2014-10-06 at 17:54 -0400, David Miller wrote:
> Ok I'm sold on your patch then if two major chipsets already benefit
> from differing values.
>
> I'll apply this, thanks Eric.
I did the experiment on I40e, and no difference on this NIC.
(Note it stills lacks xmit_more support atm)
Thanks David
^ permalink raw reply
* net-next is CLOSED...
From: David Miller @ 2014-10-06 22:25 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, linux-wireless
We are in a sort-of state of limbo with 3.17 having been released the
other day and the actual merge window integration happening next week
when Linus returns from some travels.
Please do not submit new feature patches until I open net-next back
up which will probably be in just over 2 weeks from now.
I understand that the ->xmit_more batching stuff might still need
some fixes and/or heuristic tweaks and that's fine.
Please direct your efforts to stabilizing things over the next
week or two, thanks!
^ permalink raw reply
* [net-next] openvswitch: fix a compilation error when CONFIG_INET is not setW!
From: Andy Zhou @ 2014-10-06 22:15 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou
Fix a openvswitch compilation error when CONFIG_INET is not set:
=====================================================
In file included from include/net/geneve.h:4:0,
from net/openvswitch/flow_netlink.c:45:
include/net/udp_tunnel.h: In function 'udp_tunnel_handle_offloads':
>> include/net/udp_tunnel.h:100:2: error: implicit declaration of function 'iptunnel_handle_offloads' [-Werror=implicit-function-declaration]
>> return iptunnel_handle_offloads(skb, udp_csum, type);
>> ^
>> >> include/net/udp_tunnel.h:100:2: warning: return makes pointer from integer without a cast
>> >> cc1: some warnings being treated as errors
=====================================================
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---
drivers/net/Kconfig | 1 -
include/net/geneve.h | 36 +++++++++++++++++++++---------------
net/ipv4/Kconfig | 29 +++++++++++++++--------------
3 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c6f6f69..4706386 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -147,7 +147,6 @@ config MACVTAP
config VXLAN
tristate "Virtual eXtensible Local Area Network (VXLAN)"
depends on INET
- select NET_IP_TUNNEL
select NET_UDP_TUNNEL
---help---
This allows one to create vxlan virtual interfaces that provide
diff --git a/include/net/geneve.h b/include/net/geneve.h
index ce98865..112132c 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -1,22 +1,10 @@
#ifndef __NET_GENEVE_H
#define __NET_GENEVE_H 1
+#ifdef CONFIG_INET
#include <net/udp_tunnel.h>
+#endif
-struct geneve_sock;
-
-typedef void (geneve_rcv_t)(struct geneve_sock *gs, struct sk_buff *skb);
-
-struct geneve_sock {
- struct hlist_node hlist;
- geneve_rcv_t *rcv;
- void *rcv_data;
- struct work_struct del_work;
- struct socket *sock;
- struct rcu_head rcu;
- atomic_t refcnt;
- struct udp_offload udp_offloads;
-};
/* Geneve Header:
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -74,6 +62,22 @@ struct genevehdr {
struct geneve_opt options[];
};
+#ifdef CONFIG_INET
+struct geneve_sock;
+
+typedef void (geneve_rcv_t)(struct geneve_sock *gs, struct sk_buff *skb);
+
+struct geneve_sock {
+ struct hlist_node hlist;
+ geneve_rcv_t *rcv;
+ void *rcv_data;
+ struct work_struct del_work;
+ struct socket *sock;
+ struct rcu_head rcu;
+ atomic_t refcnt;
+ struct udp_offload udp_offloads;
+};
+
#define GENEVE_VER 0
#define GENEVE_BASE_HLEN (sizeof(struct udphdr) + sizeof(struct genevehdr))
@@ -88,4 +92,6 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
__u8 ttl, __be16 df, __be16 src_port, __be16 dst_port,
__be16 tun_flags, u8 vni[3], u8 opt_len, u8 *opt,
bool xnet);
-#endif
+#endif /*ifdef CONFIG_INET */
+
+#endif /*ifdef__NET_GENEVE_H */
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index c203544..e682b48 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -309,6 +309,7 @@ config NET_IPVTI
config NET_UDP_TUNNEL
tristate
+ select NET_IP_TUNNEL
default n
config NET_FOU
@@ -321,6 +322,20 @@ config NET_FOU
network mechanisms and optimizations for UDP (such as ECMP
and RSS) can be leveraged to provide better service.
+config GENEVE
+ tristate "Generic Network Virtualization Encapsulation (Geneve)"
+ depends on INET
+ select NET_UDP_TUNNEL
+ ---help---
+ This allows one to create Geneve virtual interfaces that provide
+ Layer 2 Networks over Layer 3 Networks. Geneve is often used
+ to tunnel virtual network infrastructure in virtualized environments.
+ For more information see:
+ http://tools.ietf.org/html/draft-gross-geneve-01
+
+ To compile this driver as a module, choose M here: the module
+
+
config INET_AH
tristate "IP: AH transformation"
select XFRM_ALGO
@@ -453,20 +468,6 @@ config TCP_CONG_BIC
increase provides TCP friendliness.
See http://www.csc.ncsu.edu/faculty/rhee/export/bitcp/
-config GENEVE
- tristate "Generic Network Virtualization Encapsulation (Geneve)"
- depends on INET
- select NET_IP_TUNNEL
- select NET_UDP_TUNNEL
- ---help---
- This allows one to create Geneve virtual interfaces that provide
- Layer 2 Networks over Layer 3 Networks. Geneve is often used
- to tunnel virtual network infrastructure in virtualized environments.
- For more information see:
- http://tools.ietf.org/html/draft-gross-geneve-01
-
- To compile this driver as a module, choose M here: the module
-
config TCP_CONG_CUBIC
tristate "CUBIC TCP"
default y
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: David Miller @ 2014-10-06 22:19 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel, john.fastabend
In-Reply-To: <1412633064.2916.46.camel@joe-AO725>
From: Joe Perches <joe@perches.com>
Date: Mon, 06 Oct 2014 15:04:24 -0700
> On Mon, 2014-10-06 at 17:56 -0400, David Miller wrote:
>> From: Joe Perches <joe@perches.com>
>> Date: Mon, 06 Oct 2014 14:51:38 -0700
>>
>> > Reduce noise when compiling W=1.
> []
>> BTW, this patch reminds me that if people think there are
>> subdirectories where we can turn on things like -Werror in the
>> networking I would be very happy to apply such patches.
> []
>> Things like net/core/ for example should be doable for sure.
>
> I don't have any significant opposition to -Werror, but
> I think there are real arguments _against_ using -Werror.
>
> I think the primary one is new compiler versions have a
> tendency to add new warnings for various things that can
> unnecessarily and unpredictably break the build.
My experience over at least a decade with arch/sparc doesn't match
your claims.
^ permalink raw reply
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: David Miller @ 2014-10-06 22:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1412619987.11091.76.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 06 Oct 2014 11:26:27 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Marking this as static allows compiler to inline it.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ 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