* Re: [PATCH iproute2] iplink: new option to set neigh suppression on a bridge port
From: Stephen Hemminger @ 2017-10-11 17:58 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, nikolay
In-Reply-To: <1507610533-18724-1-git-send-email-roopa@cumulusnetworks.com>
On Mon, 9 Oct 2017 21:42:13 -0700
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> neigh suppression can be used to suppress arp and nd flood
> to bridge ports. It maps to the recently added
> kernel support for bridge port flag IFLA_BRPORT_NEIGH_SUPPRESS.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Applied to net-next branch.
^ permalink raw reply
* Re: [PATCH iproute2 net-next] ip: mroute: Print offload indication
From: Stephen Hemminger @ 2017-10-11 17:55 UTC (permalink / raw)
To: Yotam Gigi; +Cc: netdev, davem, mlxsw
In-Reply-To: <20171008144304.48850-1-yotamg@mellanox.com>
On Sun, 8 Oct 2017 17:43:04 +0300
Yotam Gigi <yotamg@mellanox.com> wrote:
> Since kernel net-next commit c7c0bbeae950 ("net: ipmr: Add MFC offload
> indication") the kernel indicates on an MFC entry whether it was offloaded
> using the RTNH_F_OFFLOAD flag. Update the "ip mroute show" command to
> indicate when a route is offloaded, similarly to the "ip route show"
> command.
>
> Example output:
> $ ip mroute
> (0.0.0.0, 239.255.0.1) Iif: sw1p7 Oifs: t_br0 State: resolved offload
> (192.168.1.1, 239.255.0.1) Iif: sw1p7 Oifs: sw1p4 State: resolved offload
>
> Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Looks good, applied.
Thanks.
^ permalink raw reply
* Re: [PATCH iproute2 1/1] color: Fix ip segfault in color_fprintf() when using --color switch
From: Stephen Hemminger @ 2017-10-11 17:53 UTC (permalink / raw)
To: Petr Vorel; +Cc: netdev, Julien Fortin
In-Reply-To: <20171008143847.21699-1-petr.vorel@gmail.com>
On Sun, 8 Oct 2017 16:38:47 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:
> diff --git a/include/json_print.h b/include/json_print.h
> index b6ce1f9f..2f3f07c8 100644
> --- a/include/json_print.h
> +++ b/include/json_print.h
> @@ -53,7 +53,7 @@ void close_json_array(enum output_type type, const char *delim);
> const char *fmt, \
> type value) \
> { \
> - print_color_##type_name(t, -1, key, fmt, value); \
> + print_color_##type_name(t, 0, key, fmt, value); \
> }
> _PRINT_FUNC(int, int);
> _PRINT_FUNC(bool, bool);
> diff --git a/lib/color.c b/lib/color.c
> index 79d5e289..e597798f 100644
> --- a/lib/color.c
> +++ b/lib/color.c
> @@ -110,7 +110,7 @@ int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
> }
>
> ret += fprintf(fp, "%s",
> - color_codes[attr_colors[is_dark_bg ? attr + 8 : attr]]);
> + color_codes[attr_colors[is_dark_bg ? attr + 6 : attr - 1]]);
Magic offsets (8 and -1) are error prone. Can this be changed to an enum value from colors?
^ permalink raw reply
* Re: [PATCH iproute2 v2 0/3] ss: add AF_VSOCK support
From: Stephen Hemminger @ 2017-10-11 17:52 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: netdev, Jorgen Hansen, Dexuan Cui
In-Reply-To: <20171006154841.10495-1-stefanha@redhat.com>
On Fri, 6 Oct 2017 11:48:38 -0400
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> v2:
> * Use uint64_t instead of __u64 for filter->families
> * Added reference to net-next commit that merged vsock_diag.ko
>
> This patch series adds AF_VSOCK support to ss(8). AF_VSOCK is a host<->guest
> communications channel supported by VMware, KVM (virtio-vsock), and Hyper-V.
>
> To dump AF_VSOCK sockets:
>
> $ ss --vsock
>
> The vsock_diag.ko module has now been merged in the Linux net-next tree. I
> have verified that the <linux/vm_sockets_diag.h> header copy in this patch
> series is in sync with Linux net-next. See commit
> 5820299a271fd3dc9b1733e1e10cd7b983edd028 ("Merge branch 'VSOCK-sock_diag'").
>
> Stefan Hajnoczi (3):
> ss: allow AF_FAMILY constants >32
> include: add <linux/vm_sockets_diag.h>
> ss: add AF_VSOCK support
>
> include/linux/vm_sockets_diag.h | 33 ++++++
> misc/ss.c | 238 +++++++++++++++++++++++++++++++++++-----
> man/man8/ss.8 | 8 +-
> 3 files changed, 249 insertions(+), 30 deletions(-)
> create mode 100644 include/linux/vm_sockets_diag.h
>
Thanks Stefan.
Applied to iproute2 net-next branch.
Headers got rearranged recently.
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Joe Perches @ 2017-10-11 17:47 UTC (permalink / raw)
To: David Laight, Gustavo A. R. Silva, Jes Sorensen, Kalle Valo
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD009123C@AcuExch.aculab.com>
On Wed, 2017-10-11 at 12:54 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 11 October 2017 11:21
> > On Tue, 2017-10-10 at 14:30 -0500, Gustavo A. R. Silva wrote:
> > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > > where we are expecting to fall through.
> >
> > perhaps use Arnaldo's idea:
> >
> > https://lkml.org/lkml/2017/2/9/845
> > https://lkml.org/lkml/2017/2/10/485
>
> gah, that is even uglier and requires a chase through
> headers to find out what it means.
Sure, if you think __fallthrough; isn't self-documenting.
case foo;
bar;
__fallthrough;
case baz;
etc...
^ permalink raw reply
* Re: [jkirsher/next-queue PATCH v4 0/6] tc-flower based cloud filters in i40e
From: Alexander Duyck @ 2017-10-11 17:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: Amritha Nambiar, intel-wired-lan, Jeff Kirsher,
Duyck, Alexander H, Netdev, Jamal Hadi Salim, Cong Wang
In-Reply-To: <20171011125635.GD2039@nanopsycho>
On Wed, Oct 11, 2017 at 5:56 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Oct 11, 2017 at 02:24:12AM CEST, amritha.nambiar@intel.com wrote:
>>This patch series enables configuring cloud filters in i40e
>>using the tc-flower classifier. The classification function
>>of the filter is to match a packet to a class. cls_flower is
>>extended to offload classid to hardware. The offloaded classid
>>is used direct matched packets to a traffic class on the device.
>>The approach here is similar to the tc 'prio' qdisc which uses
>>the classid for band selection. The ingress qdisc is called ffff:0,
>>so traffic classes are ffff:1 to ffff:8 (i40e has max of 8 TCs).
>
>
> NACK. This clearly looks like abuse of classid to something
> else. Classid is here to identify qdisc instance. However, you use it
> for hw tclass identification. This is mixing of apples and oranges.
>
> Why?
>
> Please don't try to abuse things! This is not nice.
This isn't an abuse. This is reproducing in hardware what is already
the behavior for software. Isn't that how offloads are supposed to
work?
This is exactly how prio currently handles this. We are essentially
doing the exact same thing in the hardware where we are choosing a
queueing group based on the class ID. You could setup a prio qdisc. If
you are offloading a qdisc behavior into hardware how are you supposed
to emulate the behavior if you aren't allowing the offload to use the
same mechanism?
- Alex
^ permalink raw reply
* Re: [PATCH iproute2] iproute: build more easily on Android
From: Stephen Hemminger @ 2017-10-11 17:45 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, enh
In-Reply-To: <20171002170337.42235-1-lorenzo@google.com>
On Tue, 3 Oct 2017 02:03:37 +0900
Lorenzo Colitti <lorenzo@google.com> wrote:
> iproute2 contains a bunch of kernel headers, including uapi ones.
> Android's libc uses uapi headers almost directly, and uses a
> script to fix kernel types that don't match what userspace
> expects.
>
> For example: https://issuetracker.google.com/36987220 reports
> that our struct ip_mreq_source contains "__be32 imr_multiaddr"
> rather than "struct in_addr imr_multiaddr". The script addresses
> this by replacing the uapi struct definition with a #include
> <bits/ip_mreq.h> which contains the traditional userspace
> definition.
>
> Unfortunately, when we compile iproute2, this definition
> conflicts with the one in iproute2's linux/in.h.
>
> Historically we've just solved this problem by running "git rm"
> on all the iproute2 include/linux headers that break Android's
> libc. However, deleting the files in this way makes it harder to
> keep up with upstream, because every upstream change to
> an include file causes a merge conflict with the delete.
>
> This patch fixes the problem by moving the iproute2 linux headers
> from include/linux to include/uapi/linux.
>
> Tested: compiles on ubuntu trusty (glibc)
>
> Signed-off-by: Elliott Hughes <enh@google.com>
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
>
I went ahead and did this for 4.14 (and net-next).
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] net sched actions: fix module auto-loading
From: Cong Wang @ 2017-10-11 17:30 UTC (permalink / raw)
To: Roman Mashak
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim
In-Reply-To: <1507733430-17860-3-git-send-email-mrv@mojatatu.com>
On Wed, Oct 11, 2017 at 7:50 AM, Roman Mashak <mrv@mojatatu.com> wrote:
> Macro __stringify_1() can stringify a macro argument, however IFE_META_*
> are enums, so they never expand, however request_module expects an integer
> in IFE module name, so as a result it always fails to auto-load.
>
> Fixes: ef6980b6becb ("introduce IFE action")
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Good catch!
Alternatively, it seems we can also do this:
...
#define _IFE_META_PRIO 3
enum {
...
IFE_META_PRIO = _IFE_META_PRIO,
...
};
#define MODULE_ALIAS_IFE_META(metan) MODULE_ALIAS("ifemeta" ##_
__stringify_1(metan))
But it does _not_ look any better than yours.
So,
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: pull-request: mac80211-next 2017-10-11
From: David Miller @ 2017-10-11 17:15 UTC (permalink / raw)
To: johannes-cdvu00un1VgdHxzADdlk8Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171011123613.28890-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Wed, 11 Oct 2017 14:36:12 +0200
> Here's a -next pull request. The only bigger thing here is the
> addition of the regulatory database as firmware, which will
> allow us to - over time - get rid of CRDA, as well as having
> the option of adding more fields to the database where needed,
> this would've been extremely complex with CRDA because it had
> not been built with extensibility in mind.
>
> Please pull and let me know if there's any problem.
Pulled, thanks!
^ permalink raw reply
* Re: [PATCH v2] XDP Program for Ip forward
From: Christina Jacob @ 2017-10-11 17:06 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, linux-kernel, linux-arm-kernel, Sunil.Goutham, daniel,
David Ahern, Christina Jacob
In-Reply-To: <20171010160057.32678367@redhat.com>
On Tue, Oct 10, 2017 at 7:30 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Tue, 10 Oct 2017 15:12:31 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> > I'll try to test/benchmark your program...
>
> In my initial testing, I cannot get this to work...
>
What is the test setup you are using? So that I can also test
I verified the program in this minimal test setup, I did not see any
issue with the mac addresses being set.
Below is the test setup.
machine 1 machine 2
(90.0.0.2)port 1 ===============>port3(90.0.0.1)
||
(80.0.0.2)port 2<================port4(80.0.0.1)
traffic gen xdp program
Below are the steps followed to run the program.
Assigned ips in different subnets to interfaces 3 and 4
Ip of port 1 is in the same subnet as port 3
Ip of port 2 is in the same subnet as port 4
sysctl -w net.core.bpf_jit_enable=1
./xdp_router_ipv4 eth0 eth1
Note: The program will not generate arp requests. Proper arp entries
need to be added to get the packets forwarded correctly.
> You do seem to XDP_REDIRECT out the right interface, but you have an
> error with setting the correct MAC address.
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Kees Cook @ 2017-10-11 17:00 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Jes Sorensen, Kalle Valo, linux-wireless, Network Development,
LKML
In-Reply-To: <20171011093248.Horde.Jwmh0VKhmCeOxBgQzLLpYeZ@gator4166.hostgator.com>
On Wed, Oct 11, 2017 at 7:32 AM, Gustavo A. R. Silva
<garsilva@embeddedor.com> wrote:
> Quoting Jes Sorensen <jes.sorensen@gmail.com>:
>> On 10/11/2017 04:41 AM, Kalle Valo wrote:
>>> Jes Sorensen <jes.sorensen@gmail.com> writes:
>>>> On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
>>>>>
>>>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>>>> where we are expecting to fall through.
>>>>
>>>> While this isn't harmful, to me this looks like pointless patch churn
>>>> for zero gain and it's just ugly.
>>>
>>> In general I find it useful to mark fall through cases. And it's just a
>>> comment with two words, so they cannot hurt your eyes that much.
>>
>> I don't see them being harmful in the code, but I don't see them of much
>> use either. If it happened as part of natural code development, fine. My
>> objection is to people running around doing this systematically causing
>> patch churn for little to zero gain.
>
> I understand that you think this is of zero gain for you, but as Florian
> Fainelli pointed out:
>
> "That is the canonical way to tell static analyzers and compilers that
> fall throughs are wanted and not accidental mistakes in the code. For
> people that deal with these kinds of errors, it's quite helpful, unless
> you suggest disabling that particular GCC warning specific for that
> file/directory?"
>
> this is very helpful for people working on fixing issues reported by static
> analyzers. It saves a huge amount of time when dealing with False Positives.
> Also, there are cases when an apparently intentional fall-through turns out
> to be an actual missing break or continue.
>
> So there is an ongoing effort to detect such cases and avoid them to show up
> in the future by at least warning people about a potential issue in their
> code. And this is helpful for everybody.
This is an unfortunate omission in the C language, and thankfully both
gcc and clang have stepped up to solve this the same way static
analyzers have solved it. It's not exactly pretty, but it does both
document the intention for humans and provide a way for analyzers to
report issues. Having the compiler help us not make mistakes is quite
handy, and with Gustavo grinding through all the Coverity warnings,
he's found actual bugs with missing "break"s, so I think this has a
demonstrable benefit to the code-base as a whole. It makes things
unambiguous to someone else reviewing the code.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Ethtool question
From: Ben Greear @ 2017-10-11 16:51 UTC (permalink / raw)
To: netdev
I noticed today that setting some ethtool settings to the same value
returns an error code. I would think this should silently return
success instead? Makes it easier to call it from scripts this way:
[root@lf0313-6477 lanforge]# ethtool -L eth3 combined 1
combined unmodified, ignoring
no channel parameters changed, aborting
current values: tx 0 rx 0 other 1 combined 1
[root@lf0313-6477 lanforge]# echo $?
1
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [patch net-next 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net
From: Cong Wang @ 2017-10-11 16:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Saeed Mahameed, matanb, leonro, mlxsw
In-Reply-To: <20171010211926.GL2033@nanopsycho>
On Tue, Oct 10, 2017 at 2:19 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Oct 10, 2017 at 07:44:53PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Tue, Oct 10, 2017 at 12:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> -static int tcf_mirred_device(const struct tc_action *a, struct net *net,
>>> - struct net_device **mirred_dev)
>>> +static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
>>> {
>>> - int ifindex = tcf_mirred_ifindex(a);
>>> + struct tcf_mirred *m = to_mirred(a);
>>>
>>> - *mirred_dev = __dev_get_by_index(net, ifindex);
>>> - if (!*mirred_dev)
>>> - return -EINVAL;
>>> - return 0;
>>> + return __dev_get_by_index(m->net, m->tcfm_ifindex);
>>
>>Hmm, why not just return m->tcfm_dev?
>
> I just follow the existing code. The change you suggest should be a
> separate follow-up patch.
Why?
Your goal is "make tc_action_ops->get_dev return dev and avoid passing net",
using m->tcfm_dev is simpler and could save you from adding a net pointer
to struct tcf_mirred too.
^ permalink raw reply
* Re: BUG:af_packet fails to TX TSO frames
From: Anton Ivanov @ 2017-10-11 16:32 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, David Miller
In-Reply-To: <CAF=yD-+JaS8JwEC2TyUG=Ebaho-MM+gjumrbHYM-H0EK1Hu62Q@mail.gmail.com>
Working through it at the moment.
The validation logic is prohibiting what the hardware considers to be a
perfectly legit skb.
Once I narrow down the culprit I will come back with my findings.
Thanks for pointing me where to look by the way.
A.
On 10/11/17 17:26, Willem de Bruijn wrote:
> On Wed, Oct 11, 2017 at 11:54 AM, Anton Ivanov
> <anton.ivanov@cambridgegreys.com> wrote:
>> It is that patch.
>>
>> I rolled it back and immediately got it to work correctly on a Broadcom
>> Tigon. I can test on all other scenarios, I have tried, I suspect it will
>> come back alive on all of them.
>>
>> I am going to try to trace it through and see exactly where it drops a skb
>> which the card has no issues in accepting.
> It might be in the initialization of gso_type and csum. The virtio_net_hdr
> can encode various combinations of flags that are not allowed by the
> validation logic.
>
--
Anton R. Ivanov
Cambridge Greys Limited, England and Wales company No 10273661
http://www.cambridgegreys.com/
^ permalink raw reply
* Re: [PATCH net-next 2/2] net sched act_vlan: VLAN action rewrite to use RCU lock/unlock and update
From: Cong Wang @ 2017-10-11 16:27 UTC (permalink / raw)
To: Manish Kurup
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, LKML, Alexander Aring,
Roman Mashak, manish.kurup
In-Reply-To: <1507689219-22993-1-git-send-email-manish.kurup@verizon.com>
On Tue, Oct 10, 2017 at 7:33 PM, Manish Kurup <kurup.manish@gmail.com> wrote:
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 14c262c..9bb0236 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -29,31 +29,37 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
> int action;
> int err;
> u16 tci;
> + struct tcf_vlan_params *p;
>
> tcf_lastuse_update(&v->tcf_tm);
> bstats_cpu_update(this_cpu_ptr(v->common.cpu_bstats), skb);
>
> - spin_lock(&v->tcf_lock);
> - action = v->tcf_action;
> -
spin_lock() is removed here, see below.
> /* Ensure 'data' points at mac_header prior calling vlan manipulating
> * functions.
> */
> if (skb_at_tc_ingress(skb))
> skb_push_rcsum(skb, skb->mac_len);
>
> - switch (v->tcfv_action) {
> + rcu_read_lock();
> +
> + action = READ_ONCE(v->tcf_action);
> +
> + p = rcu_dereference(v->vlan_p);
> +
> + switch (p->tcfv_action) {
> case TCA_VLAN_ACT_POP:
> err = skb_vlan_pop(skb);
> if (err)
> goto drop;
> break;
> +
> case TCA_VLAN_ACT_PUSH:
> - err = skb_vlan_push(skb, v->tcfv_push_proto, v->tcfv_push_vid |
> - (v->tcfv_push_prio << VLAN_PRIO_SHIFT));
> + err = skb_vlan_push(skb, p->tcfv_push_proto, p->tcfv_push_vid |
> + (p->tcfv_push_prio << VLAN_PRIO_SHIFT));
> if (err)
> goto drop;
> break;
> +
> case TCA_VLAN_ACT_MODIFY:
> /* No-op if no vlan tag (either hw-accel or in-payload) */
> if (!skb_vlan_tagged(skb))
> @@ -69,15 +75,16 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
> goto drop;
> }
> /* replace the vid */
> - tci = (tci & ~VLAN_VID_MASK) | v->tcfv_push_vid;
> + tci = (tci & ~VLAN_VID_MASK) | p->tcfv_push_vid;
> /* replace prio bits, if tcfv_push_prio specified */
> - if (v->tcfv_push_prio) {
> + if (p->tcfv_push_prio) {
> tci &= ~VLAN_PRIO_MASK;
> - tci |= v->tcfv_push_prio << VLAN_PRIO_SHIFT;
> + tci |= p->tcfv_push_prio << VLAN_PRIO_SHIFT;
> }
> /* put updated tci as hwaccel tag */
> - __vlan_hwaccel_put_tag(skb, v->tcfv_push_proto, tci);
> + __vlan_hwaccel_put_tag(skb, p->tcfv_push_proto, tci);
> break;
> +
> default:
> BUG();
> }
> @@ -89,6 +96,7 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
> qstats_drop_inc(this_cpu_ptr(v->common.cpu_qstats));
>
> unlock:
> + rcu_read_unlock();
> if (skb_at_tc_ingress(skb))
> skb_pull_rcsum(skb, skb->mac_len);
>
But here spin_unlock() is not removed... At least it doesn't show in diff
context. It's probably unbalanced spinlock.
> @@ -111,6 +119,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
> struct nlattr *tb[TCA_VLAN_MAX + 1];
> struct tc_vlan *parm;
> struct tcf_vlan *v;
> + struct tcf_vlan_params *p, *p_old;
> int action;
> __be16 push_vid = 0;
> __be16 push_proto = 0;
> @@ -187,16 +196,33 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
>
> v = to_vlan(*a);
>
> - spin_lock_bh(&v->tcf_lock);
> -
> - v->tcfv_action = action;
> - v->tcfv_push_vid = push_vid;
> - v->tcfv_push_prio = push_prio;
> - v->tcfv_push_proto = push_proto;
> + ASSERT_RTNL();
> + p = kzalloc(sizeof(*p), GFP_KERNEL);
> + if (unlikely(!p)) {
> + if (ovr)
> + tcf_idr_release(*a, bind);
> + return -ENOMEM;
> + }
>
> v->tcf_action = parm->action;
>
> - spin_unlock_bh(&v->tcf_lock);
> + p_old = rtnl_dereference(v->vlan_p);
> +
> + if (ovr)
> + spin_lock_bh(&v->tcf_lock);
Why still take spinlock when you already have RTNL lock?
What's the point?
^ permalink raw reply
* Re: BUG:af_packet fails to TX TSO frames
From: Willem de Bruijn @ 2017-10-11 16:26 UTC (permalink / raw)
To: Anton Ivanov; +Cc: Network Development, David Miller
In-Reply-To: <844d6e0f-6ee7-74bc-b961-faa77b240303@cambridgegreys.com>
On Wed, Oct 11, 2017 at 11:54 AM, Anton Ivanov
<anton.ivanov@cambridgegreys.com> wrote:
> It is that patch.
>
> I rolled it back and immediately got it to work correctly on a Broadcom
> Tigon. I can test on all other scenarios, I have tried, I suspect it will
> come back alive on all of them.
>
> I am going to try to trace it through and see exactly where it drops a skb
> which the card has no issues in accepting.
It might be in the initialization of gso_type and csum. The virtio_net_hdr
can encode various combinations of flags that are not allowed by the
validation logic.
^ permalink raw reply
* Re: BUG:af_packet fails to TX TSO frames
From: Anton Ivanov @ 2017-10-11 15:54 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, David Miller
In-Reply-To: <f2fe3f46-2ef1-0029-1c44-3576ca19d34b@cambridgegreys.com>
It is that patch.
I rolled it back and immediately got it to work correctly on a Broadcom
Tigon. I can test on all other scenarios, I have tried, I suspect it
will come back alive on all of them.
I am going to try to trace it through and see exactly where it drops a
skb which the card has no issues in accepting.
A.
On 10/11/17 14:50, Anton Ivanov wrote:
>
>
> On 10/11/17 14:39, Willem de Bruijn wrote:
>> On Wed, Oct 11, 2017 at 4:39 AM, Anton Ivanov
>> <anton.ivanov@cambridgegreys.com> wrote:
>>> Hi all,
>>>
>>> I am having an issue with af_packet.c
>>>
>>> It fails to transmit any TSO frame submitted via raw socket + vnet
>>> headers.
>>> An identical frame is considered valid for tap.
>> This may be due to validation. As of commit 104ba78c9880 ("packet: on
>> direct_xmit, limit tso and csum to supported devices") the packet socket
>> code validates TSO packets and HW support in the direct_xmit path.
>
> I will look at it. I have tried with bridge+vEth (raw socket on the
> vEth) and directly on a tg3 and e1000e. All of these should be tso
> capable.
>
> CSUM definitely works in both cases, it is only TSO and only via raw
> socket on TX which is refusing to work.
>
>>
>> Do you have a test program or packet (incl. vnet hdr) to reproduce this
>> with? I usually test this path with
>
> My test program at present is UML instance with my vector IO patches.
> Latest version just went to the uml mailing list and can be pulled from
>
> http://foswiki.kot-begemot.co.uk/Main/EatYourOwnDogFoodOnUML
>
> I am going to dump one of the frames being produced and isolate it for
> a standalone test case.
>
>>
>> https://github.com/wdebruij/kerneltools/blob/master/tests/psock_txring_vnet.c
>>
>>
>>> The frames are generated out of legit linux skbufs (in UML) and vnet
>>> headers
>>> work for checksumming on raw, so I should have the raw
>>> initialization right.
>>>
>>> The header is supposedly parsed correctly and the newly formed skbuf
>>> is sent
>>> to the device transmit routine (or enqueued) . I have debugged it as
>>> far as
>>> it reaching the following line in packet_snd() (line 2592 in 4.13):
>>>
>>> err = po->xmit(skb);
>> That maps either on to packet_direct_xmit or dev_queue_xmit.
>
> I know. In my case it is direct_xmit as I have asked for QDISC bypass.
>
>>
>>> This returns NET_XMIT_DROP for any TSO capable device I tested.
>> You can also try
>>
>> perf record -a -g -e skb:kfree_skb sleep 10
>> perf report
>>
>> to see where these packets are dropped.
>
> Thanks, will try that.
>
>>
>>> They dislike
>>> the frame. Same frame is accepted by tap. I have went through the
>>> header
>>> parsing and skb allocation code in both af_packet and tap several
>>> times and
>>> I do not see any material difference (except the new zerocopy
>>> stuff). So,
>>> frankly, I am stuck.
>>>
>>> Can someone help me to debug this. I do not see an easy way to debug
>>> it, but
>>> this is not a part of the kernel I am familiar with. Is there a
>>> suitable
>>> helper function to try to segment the frame and see exactly what is
>>> wrong
>>> with it?
>>>
>>> Cc-ing DaveM as this has no specific maintainer so it falls under his
>>> umbrella remit.
>>>
>>> --
>>> Anton R. Ivanov
>>>
>>> Cambridge Greys Limited, England and Wales company No 10273661
>>> http://www.cambridgegreys.com/
>>>
>
--
Anton R. Ivanov
Cambridge Greys Limited, England and Wales company No 10273661
http://www.cambridgegreys.com/
^ permalink raw reply
* Re: [PATCH 0/4] RCU: introduce noref debug
From: Paul E. McKenney @ 2017-10-11 15:45 UTC (permalink / raw)
To: Paolo Abeni
Cc: linux-kernel, Josh Triplett, Steven Rostedt, David S. Miller,
Eric Dumazet, Hannes Frederic Sowa, netdev
In-Reply-To: <1507733436.2487.32.camel@redhat.com>
On Wed, Oct 11, 2017 at 04:50:36PM +0200, Paolo Abeni wrote:
> On Tue, 2017-10-10 at 21:02 -0700, Paul E. McKenney wrote:
> > Linus and Ingo will ask me how users decide how they should set that
> > additional build flag. Especially given that if there is code that
> > requires non-strict checking, isn't everyone required to set up non-strict
> > checking to avoid false positives? Unless you can also configure out
> > all the code that requires non-strict checking, I suppose, but how
> > would you keep track of what to configure out?
>
> I'm working to a new version using a single compile flag - without
> additional strict option.
>
> I don't know of any other subsytem that stores rcu pointer in
> datastructures for a longer amount of time. That having said, I wonder
> if the tests should completely move to the networking subsystem for the
> time being. The Kconfig option would thus be called NET_DEBUG or
> something along the lines. For abstraction it would be possible to add
> an atomic_notifier_chain to the rcu_read/unlock methods, where multiple
> users or checkers could register for. That way we keep the users
> seperate from the implementation for the cost of a bit more layering
> and more indirect calls. But given that this will anyway slow down
> execution a lot, it will anyway only be suitable in
> testing/verification/debugging environments.
I like this approach. And if it does a good job of finding networking
bugs over the next year or so, I would be quite happy to consider
something for all RCU users.
> > OK. There will probably be some discussion about the API in that case.
>
> I'll drop noref parameter, the key will became mandatory - the exact
> position of where the reference of RCU managed object is stored. In the
> case of noref dst it is &skb->_skb_refdst. With this kind of API it
> should suite more subsystems.
Interesting. Do you intend to allow rcu_track_noref() to refuse to
record a pointer? Other than for the array-full case.
> > True enough. Except that if people were good about always clearing the
> > pointer, then the pointer couldn't leak, right? Or am I missing something
> > in your use cases?
>
> This is correct. The dst_entry checking in skb, which this patch series
> implements there are strict brackets in the sense of skb_dst_set,
> skb_dst_set_noref, skb_dst_force, etc., which form brackets around the
> safe uses of those dst_entries. This patch series validates that the
> correct skb_dst_* functions are being called before the sk_buff leaves
> the rcu protected section. Thus we don't need to modify and review a
> lot of code but we can just patch into those helpers already.
Makes sense. Those willing to strictly bracket uses gain some debug
assist.
> > Or to put it another way -- have you been able to catch any real
> > pointer-leak bugs with thister-leak bugs with this? The other RCU
> > debug options have had pretty long found-bug lists before we accepted
> > them.
>
> There have been two problems found so far, one is a rather minor one
> while the other one seems like a normal bug. The patches for those are
> part of this series (3/4 and 4/4).
I agree that you have started gathering evidence and that the early
indications are positive, if quite mildly so. If this time next year
there are a few tens of such bugs found, preferably including some
serious ones, I would very likely look favorably on pulling this in to
allow others to use it.
Thanx, Paul
^ permalink raw reply
* Re: [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier
From: David Ahern @ 2017-10-11 15:19 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <20171011135405.GB11653@splinter>
On 10/11/17 7:54 AM, Ido Schimmel wrote:
>> @@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
>> int err;
>>
>> register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
>> + register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
>> register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
>> + register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
>> register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
>> register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
> Need unregister in rollback and __exit function.
>
oops, thanks for noticing that. Fixed and took care of your other comments.
^ permalink raw reply
* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
From: Ido Schimmel @ 2017-10-11 15:10 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <72ad213d-7074-e4ca-6799-94da01c38f8e@gmail.com>
On Wed, Oct 11, 2017 at 09:07:20AM -0600, David Ahern wrote:
> On 10/11/17 8:13 AM, Ido Schimmel wrote:
> > On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
> >> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
> >> - u32 tb_id)
> >> + u32 tb_id,
> >> + struct netlink_ext_ack *extack)
> >> {
> >> struct mlxsw_sp_vr *vr;
> >> int err;
> >>
> >> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
> >> - if (!vr)
> >> + if (!vr) {
> >> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
> >
> > Maybe:
> > "spectrum: Exceeded number of supported VRF devices"
>
> In this context the overflow is virtual routers in spectrum as opposed
> to VRF devices in the kernel context. The existence of the VRF device
> has no bearing until a port is enslaved to it.
>
> How about:
> "spectrum: Exceeded number of supported virtual routers"
OK.
^ permalink raw reply
* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
From: David Ahern @ 2017-10-11 15:07 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <20171011141330.GC11653@splinter>
On 10/11/17 8:13 AM, Ido Schimmel wrote:
> On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
>> Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
>> to set an error message on RIF or VR overflow. Now an overflow of
>> either resource the use gets an informative message as opposed to
>
> s/the/in/ ?
'use' is supposed to be 'user'
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> index 7d53fdf2c0a8..ec4d313b9eca 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> @@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
>> }
>>
>> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
>> - u32 tb_id)
>> + u32 tb_id,
>> + struct netlink_ext_ack *extack)
>> {
>> struct mlxsw_sp_vr *vr;
>> int err;
>>
>> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
>> - if (!vr)
>> + if (!vr) {
>> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
>
> Maybe:
> "spectrum: Exceeded number of supported VRF devices"
In this context the overflow is virtual routers in spectrum as opposed
to VRF devices in the kernel context. The existence of the VRF device
has no bearing until a port is enslaved to it.
How about:
"spectrum: Exceeded number of supported virtual routers"
^ permalink raw reply
* Skiers List
From: Lorraine Jackson @ 2017-10-11 14:17 UTC (permalink / raw)
To: netdev
Hi,
Greeting of the day!
I hope this note finds you well!
Would you be interested in acquiring an email list of "Skiers" from USA?
Our Databases:- => Fishing Enthusiasts List => Outdoor Enthusiasts List
=> Cruise Travelers List => Power Boat Owners List
=> Camping Enthusiasts List => Sail Boat Owners List
=> Hiking Enthusiasts List => Timeshare Owners List
=> Cycling Enthusiasts List => Travelers List
=> Spa and Resort Visitors List and more.
Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address.
Let me know if you'd be interested in hearing more about it.
Waiting for your valuable and sincere reply.
Best Regards,
Lorraine Jackson
Business Development & Data Specialist
^ permalink raw reply
* [PATCH net-next 2/2] net sched actions: fix module auto-loading
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1507733430-17860-1-git-send-email-mrv@mojatatu.com>
Macro __stringify_1() can stringify a macro argument, however IFE_META_*
are enums, so they never expand, however request_module expects an integer
in IFE module name, so as a result it always fails to auto-load.
Fixes: ef6980b6becb ("introduce IFE action")
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/act_ife.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 791aeee..e0bc228 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -248,6 +248,20 @@ static int ife_validate_metatype(struct tcf_meta_ops *ops, void *val, int len)
return ret;
}
+static const char *ife_meta_id2name(u32 metaid)
+{
+ switch (metaid) {
+ case IFE_META_SKBMARK:
+ return "skbmark";
+ case IFE_META_PRIO:
+ return "skbprio";
+ case IFE_META_TCINDEX:
+ return "tcindex";
+ default:
+ return "unknown";
+ }
+}
+
/* called when adding new meta information
* under ife->tcf_lock for existing action
*/
@@ -263,7 +277,7 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
if (exists)
spin_unlock_bh(&ife->tcf_lock);
rtnl_unlock();
- request_module("ife-meta-%u", metaid);
+ request_module("ife-meta-%s", ife_meta_id2name(metaid));
rtnl_lock();
if (exists)
spin_lock_bh(&ife->tcf_lock);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 1/2] net sched actions: change IFE modules alias names
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1507733430-17860-1-git-send-email-mrv@mojatatu.com>
Make style of module alias name consistent with other subsystems in kernel,
for example net devices.
Fixes: 084e2f6566d2 ("Support to encoding decoding skb mark on IFE action")
Fixes: 200e10f46936 ("Support to encoding decoding skb prio on IFE action")
Fixes: 408fbc22ef1e ("net sched ife action: Introduce skb tcindex metadata encap decap")
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
include/net/tc_act/tc_ife.h | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_meta_mark.c | 2 +-
net/sched/act_meta_skbprio.c | 2 +-
net/sched/act_meta_skbtcindex.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/tc_act/tc_ife.h b/include/net/tc_act/tc_ife.h
index 30ba459..104578f 100644
--- a/include/net/tc_act/tc_ife.h
+++ b/include/net/tc_act/tc_ife.h
@@ -40,7 +40,7 @@ struct tcf_meta_ops {
struct module *owner;
};
-#define MODULE_ALIAS_IFE_META(metan) MODULE_ALIAS("ifemeta" __stringify_1(metan))
+#define MODULE_ALIAS_IFE_META(metan) MODULE_ALIAS("ife-meta-" metan)
int ife_get_meta_u32(struct sk_buff *skb, struct tcf_meta_info *mi);
int ife_get_meta_u16(struct sk_buff *skb, struct tcf_meta_info *mi);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 8ccd358..791aeee 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -263,7 +263,7 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
if (exists)
spin_unlock_bh(&ife->tcf_lock);
rtnl_unlock();
- request_module("ifemeta%u", metaid);
+ request_module("ife-meta-%u", metaid);
rtnl_lock();
if (exists)
spin_lock_bh(&ife->tcf_lock);
diff --git a/net/sched/act_meta_mark.c b/net/sched/act_meta_mark.c
index 8289217..1e3f10e 100644
--- a/net/sched/act_meta_mark.c
+++ b/net/sched/act_meta_mark.c
@@ -76,4 +76,4 @@ static void __exit ifemark_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2015)");
MODULE_DESCRIPTION("Inter-FE skb mark metadata module");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_SKBMARK);
+MODULE_ALIAS_IFE_META("skbmark");
diff --git a/net/sched/act_meta_skbprio.c b/net/sched/act_meta_skbprio.c
index 26bf4d8..4033f9f 100644
--- a/net/sched/act_meta_skbprio.c
+++ b/net/sched/act_meta_skbprio.c
@@ -73,4 +73,4 @@ static void __exit ifeprio_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2015)");
MODULE_DESCRIPTION("Inter-FE skb prio metadata action");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_PRIO);
+MODULE_ALIAS_IFE_META("skbprio");
diff --git a/net/sched/act_meta_skbtcindex.c b/net/sched/act_meta_skbtcindex.c
index 3b35774..2ea1f26 100644
--- a/net/sched/act_meta_skbtcindex.c
+++ b/net/sched/act_meta_skbtcindex.c
@@ -76,4 +76,4 @@ static void __exit ifetc_index_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2016)");
MODULE_DESCRIPTION("Inter-FE skb tc_index metadata module");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_SKBTCINDEX);
+MODULE_ALIAS_IFE_META("tcindex");
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 0/2] Fix IFE meta modules loading
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
Adjust module alias names of IFE meta modules and fix the bug that
prevented auto-loading IFE modules in run-time.
Roman Mashak (2):
net sched actions: change IFE modules alias names
net sched actions: fix module auto-loading
include/net/tc_act/tc_ife.h | 2 +-
net/sched/act_ife.c | 16 +++++++++++++++-
net/sched/act_meta_mark.c | 2 +-
net/sched/act_meta_skbprio.c | 2 +-
net/sched/act_meta_skbtcindex.c | 2 +-
5 files changed, 19 insertions(+), 5 deletions(-)
--
1.9.1
^ 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