* Re: [PATCH net-next v2 5/5] qede: Use NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-08 22:40 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <20171208220922.GE3327@localhost.localdomain>
On Fri, Dec 8, 2017 at 2:09 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> Hi,
>
> On Thu, Dec 07, 2017 at 03:03:35AM -0500, Michael Chan wrote:
>> --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
>> +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
>> @@ -940,6 +940,9 @@ int qede_change_mtu(struct net_device *ndev, int new_mtu)
>> DP_VERBOSE(edev, (NETIF_MSG_IFUP | NETIF_MSG_IFDOWN),
>> "Configuring MTU size of %d\n", new_mtu);
>>
>> + if (new_mtu > PAGE_SIZE)
>
> I don't know the specs for this card but if it needs to fit the whole
> packet in a page, maybe it should consider the ethernet header size in
> such checks?
>
I am not changing the logic in this patch, just moving the check from
qede_alloc_sge_mem() to qede_fix_features().
Typically, the chip will also do header-data split when doing GRO_HW,
so that's probably why it is checking the MTU and not the total packet
size against page size. Ariel can confirm.
^ permalink raw reply
* Re: [PATCH v8 0/5] Add the ability to do BPF directed error injection
From: Daniel Borkmann @ 2017-12-08 23:25 UTC (permalink / raw)
To: Josef Bacik
Cc: rostedt, mingo, davem, netdev, linux-kernel, ast, kernel-team,
linux-btrfs
In-Reply-To: <20171208202424.cxvsbf3v2huiolqn@destiny>
On 12/08/2017 09:24 PM, Josef Bacik wrote:
> On Fri, Dec 08, 2017 at 04:35:44PM +0100, Daniel Borkmann wrote:
>> On 12/06/2017 05:12 PM, Josef Bacik wrote:
>>> Jon noticed that I had a typo in my _ASM_KPROBE_ERROR_INJECT macro. I went to
>>> figure out why the compiler didn't catch it and it's because it was not used
>>> anywhere. I had copied it from the trace blacklist code without understanding
>>> where it was used as cscope didn't find the original macro I was looking for, so
>>> I assumed it was some voodoo and left it in place. Turns out cscope failed me
>>> and I didn't need the macro at all, the trace blacklist thing I was looking at
>>> was for marking assembly functions as blacklisted and I have no intention of
>>> marking assembly functions as error injectable at the moment.
>>>
>>> v7->v8:
>>> - removed the _ASM_KPROBE_ERROR_INJECT since it was not needed.
>>
>> The series doesn't apply cleanly to the bpf-next tree, so one last respin with
>> a rebase would unfortunately still be required, thanks!
>
> I've rebased and let it sit in my git tree to make sure kbuild test bot didn't
> blow up, can you pull from
>
> git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git bpf-override-return
>
> or do you want me to repost the whole series? Thanks,
Yeah, the patches would need to end up on netdev, so once kbuild bot went
through fine after your rebase, please send the series.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] ip6_vti: adjust vti mtu according to mtu of output device
From: Shannon Nelson @ 2017-12-08 23:25 UTC (permalink / raw)
To: Alexey Kodanev, Steffen Klassert; +Cc: netdev, David Miller, Petr Vorel
In-Reply-To: <1d086bbe-8d90-628f-4622-2fdb5fb1405e@oracle.com>
On 12/8/2017 3:54 AM, Alexey Kodanev wrote:
> On 12/08/2017 10:02 AM, Steffen Klassert wrote:
>> On Wed, Dec 06, 2017 at 07:38:19PM +0300, Alexey Kodanev wrote:
Since you're planning to do a 2nd version anyway, can we get a couple of
the commit message issues cleaned up?
>>> LTP/udp6_ipsec_vti tests fail when sending large UDP datagrams
>>> that require fragmentation and underlying device MTU <= 1500.
s/underlying device/the underlying device has/
>>> This happens because ip6_vti sets mtu to ETH_DATA_LEN and not
>>> updating it depending on a destiantion address.
s/destiantion/destination/
>>>
>>> Futhure attempts to send UDP packets may succeed because pmtu
s/Futhure/Further/
>>> get updated on ICMPV6_PKT_TOOBIG in vti6_err().
s/get/gets/
>>>
>>> Here is the example when output device MTU set to 9000:
s/output device MTU/the output device MTU is/
Thanks,
sln
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
From: Jakub Kicinski @ 2017-12-08 23:30 UTC (permalink / raw)
To: David Ahern
Cc: Quentin Monnet, Roman Gushchin, netdev, linux-kernel, kernel-team,
ast, daniel, kafai
In-Reply-To: <77e22817-4be2-ae4f-11b7-ebc4c51f39f3@gmail.com>
On Fri, 8 Dec 2017 09:52:16 -0700, David Ahern wrote:
> On 12/8/17 8:39 AM, Quentin Monnet wrote:
> > I don't believe compatibility is an issue here, since the program and
> > its documentation come together (so they should stay in sync) and are
> > part of the kernel tree (so the tool should be compatible with the
> > kernel sources it comes with). My concern is that there is no way to
> > guess from the current description what the values for ATTACH_FLAG or
> > ATTACH_TYPE can be, without reading the source code of the program—which
> > is not exactly user-friendly.
>
> The tool should be backward and forward compatible across kernel
> versions. Running a newer command on an older kernel should fail in a
> deterministic. While the tool is in the kernel tree for ease of
> development, that should not be confused with having a direct tie to any
> kernel version.
>
> I believe man pages do include kernel version descriptions in flags
> (e.g., man 7 socket -- flags are denoted with "since Linux x.y") which
> is one way to handle it with the usual caveat that vendors might have
> backported support to earlier kernels.
Let's see if I understand correctly. We have a list of hard coded
strings which the tool will recognize (static const char * const
attach_type_strings[]). We should put that list into the man page and
help so users know what values are possible. And in the "verbose"
part of the man section mark each flag with kernel version it was
introduced in.
Roman, would you agree this is the best way forward?
^ permalink raw reply
* [PATCH] rtnetlink: fix typo in GSO max segments
From: Stephen Hemminger @ 2017-12-08 23:34 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
Fixes: 46e6b992c250 ("rtnetlink: allow GSO maximums to be set on device creation")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
net/core/rtnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 412ebf0b09c6..c688dc564b11 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2684,7 +2684,7 @@ struct net_device *rtnl_create_link(struct net *net,
if (tb[IFLA_GSO_MAX_SIZE])
netif_set_gso_max_size(dev, nla_get_u32(tb[IFLA_GSO_MAX_SIZE]));
if (tb[IFLA_GSO_MAX_SEGS])
- dev->gso_max_size = nla_get_u32(tb[IFLA_GSO_MAX_SEGS]);
+ dev->gso_max_segs = nla_get_u32(tb[IFLA_GSO_MAX_SEGS]);
return dev;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v3 net-next 4/4] bpftool: implement cgroup bpf operations
From: Jakub Kicinski @ 2017-12-08 23:46 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev, linux-kernel, kernel-team, ast, daniel, kafai,
Quentin Monnet, David Ahern
In-Reply-To: <20171208145236.12635-5-guro@fb.com>
On Fri, 8 Dec 2017 14:52:36 +0000, Roman Gushchin wrote:
> +static int list_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> +{
> + __u32 prog_ids[1024] = {0};
> + char *attach_flags_str;
> + __u32 prog_cnt, iter;
> + __u32 attach_flags;
> + char buf[16];
> + int ret;
> +
...
> + case BPF_F_ALLOW_OVERRIDE:
> + attach_flags_str = "allow_override";
> + break;
> + case 0:
> + attach_flags_str = "";
> + break;
> + default:
> + snprintf(buf, sizeof(buf), "unknown(%x)", attach_flags);
> + attach_flags_str = buf;
nit: theoretically speaking strlen("unknown()") == 9 + 8 + 1 > sizeof(buf)
so if we ever use all flags this may get truncated. I personally also
like using %x without 0x in front, but I restrained myself in bpftool
to avoid potential confusion (unknown(10) could be 10 or 16). Map flags
do put the prefix in, perhaps it would be good to keep those consistent?
> + }
> +
> + for (iter = 0; iter < prog_cnt; iter++)
> + list_bpf_prog(prog_ids[iter], attach_type_strings[type],
> + attach_flags_str);
> +
> + return 0;
> +}
> +static int do_attach(int argc, char **argv)
> +{
> + int cgroup_fd, prog_fd;
> + enum bpf_attach_type attach_type;
> + int attach_flags = 0;
> + int i;
> + int ret = -1;
nit: I was hoping you'd fix the order of variables in all functions..
> + if (argc < 4) {
> + p_err("too few parameters for cgroup attach\n");
> + goto exit;
> + }
> +
> + cgroup_fd = open(argv[0], O_RDONLY);
> + if (cgroup_fd < 0) {
> + p_err("can't open cgroup %s\n", argv[1]);
> + goto exit;
> + }
> +
> + attach_type = parse_attach_type(argv[1]);
> + if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach type\n");
> + goto exit_cgroup;
> + }
> +
> + argc -= 2;
> + argv = &argv[2];
> + prog_fd = prog_parse_fd(&argc, &argv);
> + if (prog_fd < 0)
> + goto exit_cgroup;
> +
> + for (i = 0; i < argc; i++) {
> + if (strcmp(argv[i], "allow_multi") == 0) {
> + attach_flags |= BPF_F_ALLOW_MULTI;
> + } else if (strcmp(argv[i], "allow_override") == 0) {
> + attach_flags |= BPF_F_ALLOW_OVERRIDE;
I don't feel about this strongly but as I said I was trying to follow
iproute2's conventions, and it allows aliasing. So if you type "ip a"
it will give you the first thing that starts with a, not necessarily
alphabetically, more likely in order of usefulness or order in which
things were added. IOW if "allow_" selects "allow_mutli" that's what I
would actually expect it to do..
Maybe others disagree?
> + } else {
> + p_err("unknown option: %s\n", argv[i]);
> + goto exit_cgroup;
> + }
> + }
> +
> + if (bpf_prog_attach(prog_fd, cgroup_fd, attach_type, attach_flags)) {
> + p_err("failed to attach program");
> + goto exit_prog;
> + }
> +
> + if (json_output)
> + jsonw_null(json_wtr);
> +
> + ret = 0;
> +
> +exit_prog:
> + close(prog_fd);
> +exit_cgroup:
> + close(cgroup_fd);
> +exit:
> + return ret;
> +}
> +
> +static int do_detach(int argc, char **argv)
> +{
> + int prog_fd, cgroup_fd;
> + enum bpf_attach_type attach_type;
> + int ret = -1;
nit: order here too..
> + if (argc < 4) {
> + p_err("too few parameters for cgroup detach\n");
> + goto exit;
> + }
> +
> + cgroup_fd = open(argv[0], O_RDONLY);
> + if (cgroup_fd < 0) {
> + p_err("can't open cgroup %s\n", argv[1]);
> + goto exit;
> + }
> +
> + attach_type = parse_attach_type(argv[1]);
> + if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach type");
> + goto exit_cgroup;
> + }
> +
> + argc -= 2;
> + argv = &argv[2];
> + prog_fd = prog_parse_fd(&argc, &argv);
> + if (prog_fd < 0)
> + goto exit_cgroup;
> +
> + if (bpf_prog_detach2(prog_fd, cgroup_fd, attach_type)) {
> + p_err("failed to detach program");
> + goto exit_prog;
> + }
> +
> + if (json_output)
> + jsonw_null(json_wtr);
> +
> + ret = 0;
> +
> +exit_prog:
> + close(prog_fd);
> +exit_cgroup:
> + close(cgroup_fd);
> +exit:
> + return ret;
> +}
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] veth and GSO maximums
From: Solio Sarabia @ 2017-12-08 23:50 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin, davem
In-Reply-To: <20171208.142323.2060389926267620856.davem@davemloft.net>
On Fri, Dec 08, 2017 at 02:23:23PM -0500, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Thu, 7 Dec 2017 15:40:18 -0800
>
> > This is the more general way to solving the issue of GSO limits
> > not being set correctly for containers on Azure. If a GSO packet
> > is sent to host that exceeds the limit (reported by NDIS), then
> > the host is forced to do segmentation in software which has noticeable
> > performance impact.
> >
> > The core rtnetlink infrastructure already has the messages and
> > infrastructure to allow changing gso limits. With an updated iproute2
> > the following already works:
> > # ip li set dev dummy0 gso_max_size 30000
> >
> > These patches are about making it easier with veth.
>
> Ok, this is definitely a step forward.
>
> Series applied, thanks Stephen.
Thanks.
Still not seeing the iproute2 patch though, either master or net-next.
^ permalink raw reply
* Re: [PATCH net] enic: add wq clean up budget
From: Govindarajulu Varadarajan @ 2017-12-09 0:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, govindarajulu90, benve
In-Reply-To: <20171206.140933.1944224659407579701.davem@davemloft.net>
On Wed, 6 Dec 2017, David Miller wrote:
> From: Govindarajulu Varadarajan <gvaradar@cisco.com>
> Date: Tue, 5 Dec 2017 11:14:41 -0800
>
>> In case of tx clean up, we set '-1' as budget. This means clean up until
>> wq is empty or till (1 << 32) pkts are cleaned. Under heavy load this
>> will run for long time and cause
>> "watchdog: BUG: soft lockup - CPU#25 stuck for 21s!" warning.
>>
>> This patch sets wq clean up budget to 256.
>>
>> Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
>
> This driver with all of it's indirection and layers upon layers of
> macros for queue processing is so difficult to read, and this can't
> be generating nice optimal code either...
>
> Anyways, I was walking over the driver to see if the logic is
> contributing to this.
>
> The limit you are proposing sounds unnecessary, nobody else I can
> see needs this, and that includes all of the most heavily used
> drivers under load.
I used 256 as the limit because most of the other drivers use it.
* mlx4 uses MLX4_EN_DEFAULT_TX_WORK as the tx budget in mlx4_en_process_tx_cq()
Added in commit fbc6daf19745 ("net/mlx4_en: Ignore budget on TX napi polling")
* i40e&vf uses vsi->work_limit as tx budget in i40e_clean_tx_irq(), which is
set to I40E_DEFAULT_IRQ_WORK. Added in commit
a619afe814453 ("i40e/i40evf: Add support for bulk free in Tx cleanup")
* ixgbe uses q_vector->tx.work_limit as tx budget in ixgbe_clean_tx_irq(),
which is set to IXGBE_DEFAULT_TX_WORK. Added in commit
592245559e900 ("ixgbe: Change default Tx work limit size to 256 buffers")
>
> If I had to guess I'd say that the problem is that the queue loop
> keeps sampling the head and tail pointers, where as it should just
> do that _once_ and only process that TX entries found in that
> snapshot and return to the poll() routine immedately afterwards.
The only way to know the tail pointer at the time napi is scheduled is to read
hw fetch_index register. This is discouraged by hw engineers.
We work around this by using color bit. Every cq entry has color bit. It is
either 0 or 1. Hw flips the bit when it creates a new cq entry. So every new
cq entry will have a different color bit than previous. We reach end of the
queue when previous color bit is same as current cq entry's color. i.e hw did
not flip the bit, so its not a new cq entry.
So enic driver cannot know the tail pointer at the time napi is scheduled, until
we reach the tail pointer.
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] veth and GSO maximums
From: Stephen Hemminger @ 2017-12-09 0:15 UTC (permalink / raw)
To: Solio Sarabia; +Cc: netdev, sthemmin, davem
In-Reply-To: <20171208235024.GA13336@ssarabia-MOBL7.amr.corp.intel.com>
On Fri, 8 Dec 2017 15:50:25 -0800
Solio Sarabia <solio.sarabia@intel.com> wrote:
> On Fri, Dec 08, 2017 at 02:23:23PM -0500, David Miller wrote:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Date: Thu, 7 Dec 2017 15:40:18 -0800
> >
> > > This is the more general way to solving the issue of GSO limits
> > > not being set correctly for containers on Azure. If a GSO packet
> > > is sent to host that exceeds the limit (reported by NDIS), then
> > > the host is forced to do segmentation in software which has noticeable
> > > performance impact.
> > >
> > > The core rtnetlink infrastructure already has the messages and
> > > infrastructure to allow changing gso limits. With an updated iproute2
> > > the following already works:
> > > # ip li set dev dummy0 gso_max_size 30000
> > >
> > > These patches are about making it easier with veth.
> >
> > Ok, this is definitely a step forward.
> >
> > Series applied, thanks Stephen.
>
> Thanks.
> Still not seeing the iproute2 patch though, either master or net-next.
I held off on iproute2 until kernel change is accepted upstream.
^ permalink raw reply
* Re: [PATCH V2 net-next 2/8] net: hns3: Add mailbox support to VF driver
From: Philippe Ombredanne @ 2017-12-09 0:16 UTC (permalink / raw)
To: Salil Mehta
Cc: David S. Miller, yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
lipeng321-hv44wF8Li93QT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20171208211702.20104-3-salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Sali,
On Fri, Dec 8, 2017 at 10:16 PM, Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> This patch adds the support of the mailbox to the VF driver. The
> mailbox shall be used as an interface to communicate with the
> PF driver for various purposes like {set|get} MAC related
> operations, reset, link status etc. The mailbox supports both
> synchronous and asynchronous command send to PF driver.
>
> Signed-off-by: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: lipeng <lipeng321-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (c) 2016-2017 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
Why not use the new SPDX ids?
e.g.
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (c) 2016-2017 Hisilicon Limited. */
See Linus posts and Thomas doc patches for details.
--
Cordially
Philippe Ombredanne
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] wcn36xx: Reduce spinlock in indication handler
From: Bjorn Andersson @ 2017-12-09 0:35 UTC (permalink / raw)
To: Eugene Krasnikov, Kalle Valo, Loic Poulain
Cc: wcn36xx, linux-wireless, netdev, linux-kernel
In-Reply-To: <1512725730-19188-1-git-send-email-loic.poulain@linaro.org>
The purpose of pushing indication on a list and handle these in a
separate worker is to allow the handlers to sleep. It does therefor not
make much sense to hold the queue spinlock through the entire indication
worker function.
By removing items from the queue early we don't need to hold the lock
throughout the indication worker, allowing the individual handlers to
sleep.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index fa88a2a460aa..52daae863aed 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2414,6 +2414,8 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
hal_ind_msg = list_first_entry(&wcn->hal_ind_queue,
struct wcn36xx_hal_ind_msg,
list);
+ list_del(wcn->hal_ind_queue.next);
+ spin_unlock_irqrestore(&wcn->hal_ind_lock, flags);
msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
@@ -2450,8 +2452,6 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
wcn36xx_err("SMD_EVENT (%d) not supported\n",
msg_header->msg_type);
}
- list_del(wcn->hal_ind_queue.next);
- spin_unlock_irqrestore(&wcn->hal_ind_lock, flags);
kfree(hal_ind_msg);
}
int wcn36xx_smd_open(struct wcn36xx *wcn)
--
2.15.0
^ permalink raw reply related
* Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: Yafang Shao @ 2017-12-09 0:47 UTC (permalink / raw)
To: David Miller
Cc: Marcelo Ricardo Leitner, Song Liu, Alexey Kuznetsov, yoshfuji,
Steven Rostedt, Brendan Gregg, netdev, LKML
In-Reply-To: <20171208.112825.301687304435692797.davem@davemloft.net>
2017-12-09 0:28 GMT+08:00 David Miller <davem@davemloft.net>:
> From: Yafang Shao <laoar.shao@gmail.com>
> Date: Fri, 8 Dec 2017 23:50:44 +0800
>
>> 2017-12-08 23:42 GMT+08:00 David Miller <davem@davemloft.net>:
>>> From: Yafang Shao <laoar.shao@gmail.com>
>>> Date: Fri, 8 Dec 2017 11:40:23 +0800
>>>
>>>> It will looks like these,
>>>>
>>>> if (sk->sk_protocol == IPPROTO_TCP)
>>>> __tcp_set_state(newsk, TCP_SYN_RECV);
>>>> else
>>>> newsk->sk_state = TCP_SYN_RECV;
>>>>
>>>>
>>>> if (sk->sk_protocol == IPPROTO_TCP)
>>>> __tcp_set_state(sk, TCP_CLOSE);
>>>> else
>>>> sk->sk_state = TCP_CLOSE;
>>>>
>>>> if (sk->sk_protocol == IPPROTO_TCP)
>>>> tcp_state_store(sk, state);
>>>> else
>>>> sk_state_store(sk, state);
>>>>
>>>>
>>>> Some redundant code.
>>>>
>>>> IMO, put these similar code into a wrapper is more nice.
>>>
>>> I think this discussion and how ugly this is getting shows that
>>> tracing the state transitions of a socket is perhaps not best as a TCP
>>> specific feature.
>>
>> Do you mean that tcp_set_state tracepoint should be replaced with
>> sk_set_state tracepoint and move that tracepoint to
>> trace/events/sock.h ?
>
> Yes, something like that.
>
> It will avoid all of these protocol specific checks and weird
> dependencies.
That looks like a good idea.
I will try to reimplemnet it.
Thanks
Yafang
^ permalink raw reply
* [PATCH net-next 0/4] nfp: dead code, clean ups and slight improvements
From: Jakub Kicinski @ 2017-12-09 3:37 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jakub Kicinski
Hi!
This series contains small clean ups from John and Carl, and brings
no functional changes.
John's improvements target the flower code. First he makes sure we don't
allocate space in FW request messages for MAC matches if the TC rule does
not contain any. The remaining two patches remove some dead code and
unused defines.
Carl follows up with a slight optimization to his recent ethtool FW state
dumps, byte swapping input parameters once instead of the data for every
dumped item.
Carl Heymann (1):
nfp: debug dump - decrease endian conversions
John Hurley (3):
nfp: flower: do not assume mac/mpls matches
nfp: flower: remove dead code paths
nfp: flower: remove unused defines
drivers/net/ethernet/netronome/nfp/flower/cmsg.h | 19 +-----
drivers/net/ethernet/netronome/nfp/flower/match.c | 79 ++++++++--------------
.../net/ethernet/netronome/nfp/flower/offload.c | 13 ++--
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 14 ++--
4 files changed, 44 insertions(+), 81 deletions(-)
--
2.15.1
^ permalink raw reply
* [PATCH net-next 1/4] nfp: flower: do not assume mac/mpls matches
From: Jakub Kicinski @ 2017-12-09 3:37 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, John Hurley
In-Reply-To: <20171209033704.5553-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Remove the matching of mac/mpls as a default selection. These are not
necessarily set by a TC rule (unlike the port). Previously a mac/mpls
field would exist in every match and be masked out if not used. This patch
has no impact on functionality but removes unnessary memory assignment in
the match cmsg.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/flower/offload.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 553f94f55dce..1b7c59a8b139 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -150,10 +150,15 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
return -EOPNOTSUPP;
key_layer_two = 0;
- key_layer = NFP_FLOWER_LAYER_PORT | NFP_FLOWER_LAYER_MAC;
+ key_layer = NFP_FLOWER_LAYER_PORT;
key_size = sizeof(struct nfp_flower_meta_one) +
- sizeof(struct nfp_flower_in_port) +
- sizeof(struct nfp_flower_mac_mpls);
+ sizeof(struct nfp_flower_in_port);
+
+ if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS) ||
+ dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_MPLS)) {
+ key_layer |= NFP_FLOWER_LAYER_MAC;
+ key_size += sizeof(struct nfp_flower_mac_mpls);
+ }
if (dissector_uses_key(flow->dissector,
FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 2/4] nfp: flower: remove dead code paths
From: Jakub Kicinski @ 2017-12-09 3:37 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, John Hurley
In-Reply-To: <20171209033704.5553-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Port matching is selected by default on every rule so remove check for it
and delete 'else' side of the statement. Remove nfp_flower_meta_one as now
it will not feature in the code. Rename nfp_flower_meta_two given that one
has been removed.
'Additional metadata' if statement can never be true so remove it as well.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/flower/cmsg.h | 16 +----
drivers/net/ethernet/netronome/nfp/flower/match.c | 79 ++++++++--------------
.../net/ethernet/netronome/nfp/flower/offload.c | 2 +-
3 files changed, 29 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 66070741d55f..430a3932cf4d 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -165,20 +165,6 @@ struct nfp_fl_pop_vlan {
__be16 reserved;
};
-/* Metadata without L2 (1W/4B)
- * ----------------------------------------------------------------
- * 3 2 1
- * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * | key_layers | mask_id | reserved |
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- */
-struct nfp_flower_meta_one {
- u8 nfp_flow_key_layer;
- u8 mask_id;
- u16 reserved;
-};
-
struct nfp_fl_pre_tunnel {
struct nfp_fl_act_head head;
__be16 reserved;
@@ -209,7 +195,7 @@ struct nfp_fl_set_vxlan {
* NOTE: | TCI |
* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
*/
-struct nfp_flower_meta_two {
+struct nfp_flower_meta_tci {
u8 nfp_flow_key_layer;
u8 mask_id;
__be16 tci;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index 60614d4f0e22..1f2b879e12d4 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -38,7 +38,7 @@
#include "main.h"
static void
-nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame,
+nfp_flower_compile_meta_tci(struct nfp_flower_meta_tci *frame,
struct tc_cls_flower_offload *flow, u8 key_type,
bool mask_version)
{
@@ -46,7 +46,7 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame,
struct flow_dissector_key_vlan *flow_vlan;
u16 tmp_tci;
- memset(frame, 0, sizeof(struct nfp_flower_meta_two));
+ memset(frame, 0, sizeof(struct nfp_flower_meta_tci));
/* Populate the metadata frame. */
frame->nfp_flow_key_layer = key_type;
frame->mask_id = ~0;
@@ -67,14 +67,6 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame,
}
}
-static void
-nfp_flower_compile_meta(struct nfp_flower_meta_one *frame, u8 key_type)
-{
- frame->nfp_flow_key_layer = key_type;
- frame->mask_id = 0;
- frame->reserved = 0;
-}
-
static int
nfp_flower_compile_port(struct nfp_flower_in_port *frame, u32 cmsg_port,
bool mask_version, enum nfp_flower_tun_type tun_type)
@@ -278,49 +270,32 @@ int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,
ext = nfp_flow->unmasked_data;
msk = nfp_flow->mask_data;
- if (NFP_FLOWER_LAYER_PORT & key_ls->key_layer) {
- /* Populate Exact Metadata. */
- nfp_flower_compile_meta_tci((struct nfp_flower_meta_two *)ext,
- flow, key_ls->key_layer, false);
- /* Populate Mask Metadata. */
- nfp_flower_compile_meta_tci((struct nfp_flower_meta_two *)msk,
- flow, key_ls->key_layer, true);
- ext += sizeof(struct nfp_flower_meta_two);
- msk += sizeof(struct nfp_flower_meta_two);
-
- /* Populate Exact Port data. */
- err = nfp_flower_compile_port((struct nfp_flower_in_port *)ext,
- nfp_repr_get_port_id(netdev),
- false, tun_type);
- if (err)
- return err;
-
- /* Populate Mask Port Data. */
- err = nfp_flower_compile_port((struct nfp_flower_in_port *)msk,
- nfp_repr_get_port_id(netdev),
- true, tun_type);
- if (err)
- return err;
-
- ext += sizeof(struct nfp_flower_in_port);
- msk += sizeof(struct nfp_flower_in_port);
- } else {
- /* Populate Exact Metadata. */
- nfp_flower_compile_meta((struct nfp_flower_meta_one *)ext,
- key_ls->key_layer);
- /* Populate Mask Metadata. */
- nfp_flower_compile_meta((struct nfp_flower_meta_one *)msk,
- key_ls->key_layer);
- ext += sizeof(struct nfp_flower_meta_one);
- msk += sizeof(struct nfp_flower_meta_one);
- }
- if (NFP_FLOWER_LAYER_META & key_ls->key_layer) {
- /* Additional Metadata Fields.
- * Currently unsupported.
- */
- return -EOPNOTSUPP;
- }
+ /* Populate Exact Metadata. */
+ nfp_flower_compile_meta_tci((struct nfp_flower_meta_tci *)ext,
+ flow, key_ls->key_layer, false);
+ /* Populate Mask Metadata. */
+ nfp_flower_compile_meta_tci((struct nfp_flower_meta_tci *)msk,
+ flow, key_ls->key_layer, true);
+ ext += sizeof(struct nfp_flower_meta_tci);
+ msk += sizeof(struct nfp_flower_meta_tci);
+
+ /* Populate Exact Port data. */
+ err = nfp_flower_compile_port((struct nfp_flower_in_port *)ext,
+ nfp_repr_get_port_id(netdev),
+ false, tun_type);
+ if (err)
+ return err;
+
+ /* Populate Mask Port Data. */
+ err = nfp_flower_compile_port((struct nfp_flower_in_port *)msk,
+ nfp_repr_get_port_id(netdev),
+ true, tun_type);
+ if (err)
+ return err;
+
+ ext += sizeof(struct nfp_flower_in_port);
+ msk += sizeof(struct nfp_flower_in_port);
if (NFP_FLOWER_LAYER_MAC & key_ls->key_layer) {
/* Populate Exact MAC Data. */
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 1b7c59a8b139..98fb1cba3ed9 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -151,7 +151,7 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
key_layer_two = 0;
key_layer = NFP_FLOWER_LAYER_PORT;
- key_size = sizeof(struct nfp_flower_meta_one) +
+ key_size = sizeof(struct nfp_flower_meta_tci) +
sizeof(struct nfp_flower_in_port);
if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS) ||
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 3/4] nfp: flower: remove unused defines
From: Jakub Kicinski @ 2017-12-09 3:37 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, John Hurley
In-Reply-To: <20171209033704.5553-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Delete match field defines that are not supported at this time.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/flower/cmsg.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 430a3932cf4d..d6b63c8f14da 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -50,9 +50,6 @@
#define NFP_FLOWER_LAYER_CT BIT(6)
#define NFP_FLOWER_LAYER_VXLAN BIT(7)
-#define NFP_FLOWER_LAYER_ETHER BIT(3)
-#define NFP_FLOWER_LAYER_ARP BIT(4)
-
#define NFP_FLOWER_MASK_VLAN_PRIO GENMASK(15, 13)
#define NFP_FLOWER_MASK_VLAN_CFI BIT(12)
#define NFP_FLOWER_MASK_VLAN_VID GENMASK(11, 0)
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 4/4] nfp: debug dump - decrease endian conversions
From: Jakub Kicinski @ 2017-12-09 3:37 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Carl Heymann
In-Reply-To: <20171209033704.5553-1-jakub.kicinski@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
Convert the requested dump level parameter to big-endian at the start of
nfp_net_dump_calculate_size() and nfp_net_dump_populate_buffer(), then
compare and assign it directly where needed in the traversal and prolog
code. This decreases the total number of conversions used.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index cb74602f0907..cbff0adad235 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -126,13 +126,13 @@ struct nfp_dump_error {
/* to track state through debug size calculation TLV traversal */
struct nfp_level_size {
- u32 requested_level; /* input */
+ __be32 requested_level; /* input */
u32 total_size; /* output */
};
/* to track state during debug dump creation TLV traversal */
struct nfp_dump_state {
- u32 requested_level; /* input param */
+ __be32 requested_level; /* input param */
u32 dumped_size; /* adds up to size of dumped data */
u32 buf_size; /* size of buffer pointer to by p */
void *p; /* current point in dump buffer */
@@ -334,7 +334,7 @@ nfp_calc_specific_level_size(struct nfp_pf *pf, struct nfp_dump_tl *dump_level,
{
struct nfp_level_size *lev_sz = param;
- if (be32_to_cpu(dump_level->type) != lev_sz->requested_level)
+ if (dump_level->type != lev_sz->requested_level)
return 0;
return nfp_traverse_tlvs(pf, dump_level->data,
@@ -348,7 +348,7 @@ s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
struct nfp_level_size lev_sz;
int err;
- lev_sz.requested_level = flag;
+ lev_sz.requested_level = cpu_to_be32(flag);
lev_sz.total_size = ALIGN8(sizeof(struct nfp_dump_prolog));
err = nfp_traverse_tlvs(pf, spec->data, spec->size, &lev_sz,
@@ -733,7 +733,7 @@ nfp_dump_specific_level(struct nfp_pf *pf, struct nfp_dump_tl *dump_level,
{
struct nfp_dump_state *dump = param;
- if (be32_to_cpu(dump_level->type) != dump->requested_level)
+ if (dump_level->type != dump->requested_level)
return 0;
return nfp_traverse_tlvs(pf, dump_level->data,
@@ -753,7 +753,7 @@ static int nfp_dump_populate_prolog(struct nfp_dump_state *dump)
if (err)
return err;
- prolog->dump_level = cpu_to_be32(dump->requested_level);
+ prolog->dump_level = dump->requested_level;
return 0;
}
@@ -764,7 +764,7 @@ int nfp_net_dump_populate_buffer(struct nfp_pf *pf, struct nfp_dumpspec *spec,
struct nfp_dump_state dump;
int err;
- dump.requested_level = dump_param->flag;
+ dump.requested_level = cpu_to_be32(dump_param->flag);
dump.dumped_size = 0;
dump.p = dest;
dump.buf_size = dump_param->len;
--
2.15.1
^ permalink raw reply related
* RE: [PATCH net-next 1/1] net: dsa: microchip: Add Microchip KSZ8895 DSA driver
From: Tristram.Ha @ 2017-12-09 3:55 UTC (permalink / raw)
To: pavel
Cc: andrew, f.fainelli, muvarov, nathan.leigh.conrad, vivien.didelot,
UNGLinuxDriver, netdev
In-Reply-To: <20171207210448.GA16614@amd>
> Ok, let me retry:
>
> > One thing to debug this problem is to dump the MIB counters. Use the
> ethtool
> > utility to show MIB counters of both ports:
> >
> > ethtool -S lan3
> > ethtool -S eth0
> >
> > Assuming eth0 is the MAC controller that drives the switch, the receive
> counters of
> > the host port of the switch should match the transmit counters of
> > lan3, and vice versa.
>
> Hmm. I'm getting some "interesting" results from mii-tool:
>
> root@miro:~# mii-tool lan3
> lan3: negotiated 1000baseT-HD flow-control, link ok
>
> But IIRC the switch is 100mbit? And dmesg does get it right. Its just
> mii-tool that is confused.
>
> Link detection seems to work
>
> root@miro:/sys/bus/spi/devices/spi2.0# mii-tool lan1
> lan1: negotiated 1000baseT-HD flow-control, link ok
> root@miro:/sys/bus/spi/devices/spi2.0# mii-tool lan1
> lan1: no link
>
> (But that really should be 100baseT, not 1000baseT).
ethtool lan3 should also report the correct setting.
> Is there register dump available somewhere? I was using
> /sys/bus/spi/devices/spi32766.0/registers but this does not seem to be
> available.
There is a patch to add that functionality. It is very simple and I will send it
to you later. Without that it is hard to debug the DSA driver if there is
something wrong.
I also have a simple utility to communicate with that registers file to read/write
register individually. Is there a standard Linux utility for that function?
> I tried that ethtool -S, and counters do not seem to match:
> root@miro:~# ethtool -S eth0
> NIC statistics:
> tx_dropped: 0
> tx_packets: 55
> tx_broadcast: 35
> tx_multicast: 20
> tx_crc_errors: 0
> tx_undersize: 0
> tx_oversize: 0
> tx_fragment: 0
> tx_jabber: 0
> tx_collision: 0
> tx_64byte: 0
> tx_65to127byte: 35
> tx_128to255byte: 0
> tx_256to511byte: 20
> tx_512to1023byte: 0
> tx_1024to2047byte: 0
> tx_GTE2048byte: 0
> tx_octets: 9576
> IEEE_tx_drop: 0
> IEEE_tx_frame_ok: 55
> IEEE_tx_1col: 0
> IEEE_tx_mcol: 0
> IEEE_tx_def: 0
> IEEE_tx_lcol: 0
> IEEE_tx_excol: 0
> IEEE_tx_macerr: 0
> IEEE_tx_cserr: 0
> IEEE_tx_sqe: 0
> IEEE_tx_fdxfc: 0
> IEEE_tx_octets_ok: 9576
> rx_packets: 0
> rx_broadcast: 0
> rx_multicast: 0
> rx_crc_errors: 0
> rx_undersize: 0
> rx_oversize: 0
> rx_fragment: 0
> rx_jabber: 0
> rx_64byte: 0
> rx_65to127byte: 0
> rx_128to255byte: 0
> rx_256to511byte: 0
> rx_512to1023byte: 0
> rx_1024to2047byte: 0
> rx_GTE2048byte: 0
> rx_octets: 0
> IEEE_rx_drop: 0
> IEEE_rx_frame_ok: 0
> IEEE_rx_crc: 0
> IEEE_rx_align: 0
> IEEE_rx_macerr: 0
> IEEE_rx_fdxfc: 0
> IEEE_rx_octets_ok: 0
These are the MIB counters from the switch host port:
> p04_rx: 660
> p04_rx_hi: 0
> p04_rx_undersize: 0
> p04_rx_fragments: 20
This indicates a problem with the MAC. Are you using a MII or RMII version?
> p04_rx_oversize: 0
> p04_rx_jabbers: 0
> p04_rx_symbol_err: 0
> p04_rx_crc_err: 0
> p04_rx_align_err: 0
> p04_rx_mac_ctrl: 0
> p04_rx_pause: 0
> p04_rx_bcast: 0
> p04_rx_mcast: 0
> p04_rx_ucast: 0
> p04_rx_64_or_less: 0
> p04_rx_65_127: 0
> p04_rx_128_255: 0
> p04_rx_256_511: 0
> p04_rx_512_1023: 0
> p04_rx_1024_1522: 0
> p04_tx: 388
> p04_tx_hi: 0
> p04_tx_late_col: 0
> p04_tx_pause: 0
> p04_tx_bcast: 0
> p04_tx_mcast: 3
This indicates the host port tried to send frames to the MAC.
> p04_tx_ucast: 0
> p04_tx_deferred: 0
> p04_tx_total_col: 0
> p04_tx_exc_col: 0
> p04_tx_single_col: 0
> p04_tx_mult_col: 0
> p04_rx_discards: 0
> p04_tx_discards: 0
> root@miro:~# ethtool -S lan3
> NIC statistics:
> tx_packets: 24
> tx_bytes: 1356
> rx_packets: 0
> rx_bytes: 0
The previous counters are from DSA. The rest are MIB counters of the port.
> rx: 566
> rx_hi: 0
> rx_undersize: 0
> rx_fragments: 0
> rx_oversize: 0
> rx_jabbers: 0
> rx_symbol_err: 0
> rx_crc_err: 0
> rx_align_err: 0
> rx_mac_ctrl: 0
> rx_pause: 0
> rx_bcast: 0
> rx_mcast: 4
> rx_ucast: 0
> rx_64_or_less: 0
> rx_65_127: 1
> rx_128_255: 3
> rx_256_511: 0
> rx_512_1023: 0
> rx_1024_1522: 0
> tx: 0
> tx_hi: 0
> tx_late_col: 0
> tx_pause: 0
> tx_bcast: 0
> tx_mcast: 0
> tx_ucast: 0
> tx_deferred: 0
> tx_total_col: 0
> tx_exc_col: 0
> tx_single_col: 0
> tx_mult_col: 0
> rx_discards: 0
> tx_discards: 0
They just reported frames are received from the port. Because of problem with
the host port there is no transmission coming from the host port.
^ permalink raw reply
* Fund Transfer
From: Hazim Issa @ 2017-12-09 4:03 UTC (permalink / raw)
In-Reply-To: <643491189.1635315.1512792192947.ref@mail.yahoo.com>
Hello,
I'm Mr. Hazim, A banker working in a bank in my country; there is a certain deceased customer of my bank who left behind her fund.
I seek your partnership in receiving this fund. If interested, reply immediately for detailed information.
My sincere regards, Mr. Hazim Issa.
^ permalink raw reply
* Re: [PATCH v5 2/2] sock: Move the socket inuse to namespace.
From: Tonghao Zhang @ 2017-12-09 5:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Cong Wang, Eric Dumazet, Willem de Bruijn,
Linux Kernel Network Developers
In-Reply-To: <1512739489.25033.26.camel@gmail.com>
On Fri, Dec 8, 2017 at 9:24 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-12-08 at 19:29 +0800, Tonghao Zhang wrote:
>> hi all. we can add synchronize_rcu and rcu_barrier in
>> sock_inuse_exit_net to
>> ensure there are no outstanding rcu callbacks using this network
>> namespace.
>> we will not have to test if net->core.sock_inuse is NULL or not from
>> sock_inuse_add(). :)
>>
>> static void __net_exit sock_inuse_exit_net(struct net *net)
>> {
>> free_percpu(net->core.prot_inuse);
>> +
>> + synchronize_rcu();
>> + rcu_barrier();
>> +
>> + free_percpu(net->core.sock_inuse);
>> }
>
>
> Oh well. Do you have any idea of the major problem this would add ?
>
> Try the following, before and after your patches :
>
> for i in `seq 1 40`
> do
> (for j in `seq 1 100` ; do unshare -n /bin/true >/dev/null ; done) &
> done
> wait
>
> ( Check commit 8ca712c373a462cfa1b62272870b6c2c74aa83f9 )
>
Yes, I did the test. The patches drop the performance.
Before patch:
# time ./add_del_unshare.sh
net_namespace 97 125 6016 5 8 : tunables 0 0
0 : slabdata 25 25 0
real 8m19.665s
user 0m4.268s
sys 0m6.477s
After :
# time ./add_del_unshare.sh
net_namespace 102 130 6016 5 8 : tunables 0 0
0 : slabdata 26 26 0
real 8m52.563s
user 0m4.040s
sys 0m7.558s
>
> This is a complex problem, we wont accept patches that kill network
> namespaces dismantling performance by adding brute force
> synchronize_rcu() or rcu_barrier() calls.
>
> Why not freeing net->core.sock_inuse right before feeing net itself in
> net_free() ?
I try this way, alloc core.sock_inuse in net_alloc(), free it in net_free ().
It does not drop performance, and we will not always to check the
core.sock_inuse
in sock_inuse_add().
After :
# time ./add_del_unshare.sh
net_namespace 109 135 6016 5 8 : tunables 0 0
0 : slabdata 27 27 0
real 8m19.265s
user 0m4.090s
sys 0m8.185s
> You do not have to hijack sock_inuse_exit_net() just because it has a
> misleading name.
>
>
^ permalink raw reply
* Incorrect source IP address on IGMP membership report
From: Kevin Cernekee @ 2017-12-09 5:25 UTC (permalink / raw)
To: netdev
Closing a multicast socket after the final IPv4 address is deleted
from an interface will generate a membership report that uses the
source IP from a different interface. The following test script, run
from an isolated netns, reproduces the issue:
#!/bin/bash
ip link add dummy0 type dummy
ip link add dummy1 type dummy
ip link set dummy0 up
ip link set dummy1 up
ip addr add 10.1.1.1/24 dev dummy0
ip addr add 192.168.99.99/24 dev dummy1
tcpdump -U -i dummy0 -w dummy0.pcap &
socat EXEC:"sleep 2"
UDP4-DATAGRAM:239.101.1.68:8889,ip-add-membership=239.0.1.68:10.1.1.1
&
sleep 1
ip addr del 10.1.1.1/24 dev dummy0
sleep 5
kill %tcpdump
After running this script, dummy0.pcap contains one Membership Report
/ Join Group packet with source IP 10.1.1.1, and two Membership Report
/ Leave Group packets with source IP 192.168.99.99.
Sending out multicasts on the LAN using an unexpected source IP
address seems to be causing issues in some enterprise environments[0],
where the network infrastructure is set up to flag suspicious packets.
I believe the source address is provided by ip_route_output_ports()
called from igmpv3_newpack() in the kernel.
Is this behavior intentional? If not, is it something that we should fix?
I was able to suppress these reports with an iptables OUTPUT rule, but
maybe there is a cleaner way.
[0] https://bugs.chromium.org/p/chromium/issues/detail?id=786514
^ permalink raw reply
* Re: [PATCH v5 2/2] sock: Move the socket inuse to namespace.
From: Tonghao Zhang @ 2017-12-09 5:27 UTC (permalink / raw)
To: Cong Wang
Cc: Eric Dumazet, David Miller, Eric Dumazet, Willem de Bruijn,
Linux Kernel Network Developers
In-Reply-To: <CAM_iQpW=-cOVf0LNwgjaBJzMqBoBunWWyTGXYnGD=F9aB88wNA@mail.gmail.com>
On Sat, Dec 9, 2017 at 6:09 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 9:28 PM, Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>>
>> Release the netlink sock created in kernel(not hold the _net_ namespace):
>>
>
> You can avoid counting kernel sock by testing 'kern' in sk_alloc()
> and testing 'sk->sk_net_refcnt' in __sk_free().
Hi cong, if we do it in this way, we will not counter the sock created
in kernel, right ?
^ permalink raw reply
* [PATCH net-next v3 0/5] Introduce NETIF_F_GRO_HW
From: Michael Chan @ 2017-12-09 6:27 UTC (permalink / raw)
To: davem; +Cc: netdev, andrew.gospodarek
Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
hardware GRO to use the new flag.
v3:
- Let driver's ndo_fix_features() disable NETIF_F_LRO when NETIF_F_GRO_HW
is set instead of doing it in common netdev_fix_features().
v2:
- NETIF_F_GRO_HW flag propagation between upper and lower devices not
required (see patch 1).
- NETIF_F_GRO_HW depends on NETIF_F_GRO and NETIF_F_RXCSUM.
- Add dev_disable_gro_hw() to disable GRO_HW for generic XDP.
- Use ndo_fix_features() on all 3 drivers to drop GRO_HW when it is not
supported
Michael Chan (5):
net: Introduce NETIF_F_GRO_HW.
net: Disable GRO_HW when generic XDP is installed on a device.
bnxt_en: Use NETIF_F_GRO_HW.
bnx2x: Use NETIF_F_GRO_HW.
qede: Use NETIF_F_GRO_HW.
Documentation/networking/netdev-features.txt | 8 ++++++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 19 ++++++++-----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 24 +++++++++++-----
drivers/net/ethernet/qlogic/qede/qede.h | 2 ++
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 3 ++
drivers/net/ethernet/qlogic/qede/qede_filter.c | 20 ++++++++-----
drivers/net/ethernet/qlogic/qede/qede_main.c | 17 ++++-------
include/linux/netdev_features.h | 3 ++
net/core/dev.c | 36 ++++++++++++++++++++++++
net/core/ethtool.c | 1 +
11 files changed, 104 insertions(+), 33 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next v3 1/5] net: Introduce NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-09 6:27 UTC (permalink / raw)
To: davem; +Cc: netdev, andrew.gospodarek, Ariel Elior, everest-linux-l2
In-Reply-To: <1512800879-17934-1-git-send-email-michael.chan@broadcom.com>
Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
GRO. With this flag, we can now independently turn on or off hardware
GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
control hardware GRO and so it cannot be independently turned on or
off without affecting GRO.
Hardware GRO (just like GRO) guarantees that packets can be re-segmented
by TSO/GSO to reconstruct the original packet stream. It is a subset of
NETIF_F_GRO and depends on it, as well as NETIF_F_RXCSUM.
Since NETIF_F_GRO is not propagated between upper and lower devices,
NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
words, a lower device can independent have GRO/GRO_HW enabled or disabled
and no feature propagation is required. This will preserve the current
GRO behavior.
Cc: Ariel Elior <Ariel.Elior@cavium.com>
Cc: everest-linux-l2@cavium.com
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
Documentation/networking/netdev-features.txt | 8 ++++++++
include/linux/netdev_features.h | 3 +++
net/core/dev.c | 12 ++++++++++++
net/core/ethtool.c | 1 +
4 files changed, 24 insertions(+)
diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
index 7413eb0..8f36527 100644
--- a/Documentation/networking/netdev-features.txt
+++ b/Documentation/networking/netdev-features.txt
@@ -163,3 +163,11 @@ This requests that the NIC receive all possible frames, including errored
frames (such as bad FCS, etc). This can be helpful when sniffing a link with
bad packets on it. Some NICs may receive more packets if also put into normal
PROMISC mode.
+
+* rx-gro-hw
+
+This requests that the NIC enables Hardware GRO (generic receive offload).
+Hardware GRO is basically the exact reverse of TSO, and is generally
+stricter than Hardware LRO. A packet stream merged by Hardware GRO must
+be re-segmentable by GSO or TSO back to the exact original packet stream.
+Hardware GRO is dependent on GRO and RXCSUM.
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index b1b0ca7..db84c51 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -78,6 +78,8 @@ enum {
NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */
NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
+ NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
+
/*
* Add your fresh new feature above and remember to update
* netdev_features_strings[] in net/core/ethtool.c and maybe
@@ -97,6 +99,7 @@ enum {
#define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
#define NETIF_F_FSO __NETIF_F(FSO)
#define NETIF_F_GRO __NETIF_F(GRO)
+#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
#define NETIF_F_GSO __NETIF_F(GSO)
#define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
#define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
diff --git a/net/core/dev.c b/net/core/dev.c
index e32cf5c..6ebd0e7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7424,6 +7424,18 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
features &= ~dev->gso_partial_features;
}
+ if (features & NETIF_F_GRO_HW) {
+ /* Hardware GRO depends on GRO and RXCSUM. */
+ if (!(features & NETIF_F_GRO)) {
+ netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
+ features &= ~NETIF_F_GRO_HW;
+ }
+ if (!(features & NETIF_F_RXCSUM)) {
+ netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
+ features &= ~NETIF_F_GRO_HW;
+ }
+ }
+
return features;
}
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f8fcf45..50a7920 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
[NETIF_F_LLTX_BIT] = "tx-lockless",
[NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
[NETIF_F_GRO_BIT] = "rx-gro",
+ [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
[NETIF_F_LRO_BIT] = "rx-lro",
[NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 2/5] net: Disable GRO_HW when generic XDP is installed on a device.
From: Michael Chan @ 2017-12-09 6:27 UTC (permalink / raw)
To: davem; +Cc: netdev, andrew.gospodarek, Ariel Elior, everest-linux-l2
In-Reply-To: <1512800879-17934-1-git-send-email-michael.chan@broadcom.com>
Hardware should not aggregate any packets when generic XDP is installed.
Cc: Ariel Elior <Ariel.Elior@cavium.com>
Cc: everest-linux-l2@cavium.com
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
net/core/dev.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6ebd0e7..ec08ace 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1542,6 +1542,29 @@ void dev_disable_lro(struct net_device *dev)
}
EXPORT_SYMBOL(dev_disable_lro);
+/**
+ * dev_disable_gro_hw - disable HW Generic Receive Offload on a device
+ * @dev: device
+ *
+ * Disable HW Generic Receive Offload (GRO_HW) on a net device. Must be
+ * called under RTNL. This is needed if Generic XDP is installed on
+ * the device.
+ */
+static void dev_disable_gro_hw(struct net_device *dev)
+{
+ struct net_device *lower_dev;
+ struct list_head *iter;
+
+ dev->wanted_features &= ~NETIF_F_GRO_HW;
+ netdev_update_features(dev);
+
+ if (unlikely(dev->features & NETIF_F_GRO_HW))
+ netdev_WARN(dev, "failed to disable GRO_HW!\n");
+
+ netdev_for_each_lower_dev(dev, lower_dev, iter)
+ dev_disable_gro_hw(lower_dev);
+}
+
static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val,
struct net_device *dev)
{
@@ -4564,6 +4587,7 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
} else if (new && !old) {
static_key_slow_inc(&generic_xdp_needed);
dev_disable_lro(dev);
+ dev_disable_gro_hw(dev);
}
break;
--
1.8.3.1
^ 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