* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Alexander Duyck @ 2017-10-19 15:40 UTC (permalink / raw)
To: Anders K. Pedersen | Cohaesio
Cc: pstaszewski@itcare.pl, netdev@vger.kernel.org,
pavlos.parissis@gmail.com, intel-wired-lan@lists.osuosl.org,
alexander.h.duyck@intel.com
In-Reply-To: <1508415582.1186.49.camel@cohaesio.com>
On Thu, Oct 19, 2017 at 5:19 AM, Anders K. Pedersen | Cohaesio
<akp@cohaesio.com> wrote:
> Hi Alex,
>
> On ons, 2017-10-18 at 16:37 -0700, Alexander Duyck wrote:
>> When we last talked I had asked if you could do a git bisect to find
>> the memory leak and you said you would look into it. The most useful
>> way to solve this would be to do a git bisect between your current
>> kernel and the 4.11 kernel to find the point at which this started.
>> If
>> we can do that then fixing this becomes much simpler as we just have
>> to fix the patch that introduced the issue.
>
> We're also seeing a smaller memory leak (about 1 GB per day) than the
> original one even with the "Fix memory leak related filter programming
> status" fix applied. So far I've determined that the leak is present on
> 4.13.7 and was introduced between 4.11 and 4.12, so I'll do another
> round of bisection to identify the patch that introduced this.
>
> Since the router must run for a couple of hours before I can be sure
> whether a kernel is good or bad, and I can't reboot it during working
> hours, it'll probably be about a week before I have a result.
>
> --
> Venlig hilsen / Best Regards
>
> Anders K. Pedersen
> Senior Technical Manager
Anders,
I'll do some digging on my side to see if I can find any other memory
leaks that might be floating around in the driver that could have been
introduced during that time-frame.
One thing you might try that would help with your testing would be to
just disable the ATR functionality in i40e. You can do that with the
ethtool command "ethtool --set-priv-flags <iface> flow-director-atr
off". That should allow you to bisect this without needing to deal
with the "programming status" patches since you won't be programming
ATR filters which is what caused that leak.
Thanks for looking into this.
- Alex
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] net: dsa: lan9303: Add port_fast_age and port_fdb_dump methods
From: Egil Hjelmeland @ 2017-10-19 15:42 UTC (permalink / raw)
To: David Laight, 'Andrew Lunn', Vivien Didelot
Cc: f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD009C922@AcuExch.aculab.com>
On 19. okt. 2017 17:15, David Laight wrote:
> From: Andrew Lunn
>> Sent: 19 October 2017 15:15
>>>> +/* Clear learned (non-static) entry on given port */
>>>> +static void alr_loop_cb_del_port_learned(struct lan9303 *chip, u32 dat0,
>>>> + u32 dat1, int portmap, void *ctx)
>>>> +{
>>>> + int *port = ctx;
>>>
>>> You can get the value directly to make the line below more readable:
>>>
>>> int port = *(int *)ctx;
>>
>> You have to be a bit careful with this. You often see people
>> submitting patches taking away casts for void * pointers.
>> If they do that here, it should at least not compile...
>>
>> So maybe do it in two steps?
>>
>> int * pport = ctx;
>> int port = *pport;
>
> IMHO it is best to define a struct for the 'ctx and then do:
> ..., void *v_ctx)
> {
> foo_ctx *ctx = v_ctx;
> int port = ctx->port;
>
> That stops anyone having to double-check that the *(int *)
> is operating on a pointer to an integer of the correct size.
>
Does casting to a struct pointer require less manual double-check than
to a int-pointer? In neither cases the compiler can protect us, IFAIK.
But on the other hand, a the text "foo_ctx" can searched in the editor.
So in that respect it can somewhat aid to the double-checking.
So I can do that.
> One of the syntax checkers probably ought to generate a warning
> for *(integer_type *)foo since it is often a bug.
>
> David
>
>
Egil
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: Paul Moore @ 2017-10-19 15:51 UTC (permalink / raw)
To: Casey Schaufler
Cc: Richard Guy Briggs, mszeredi, Eric W. Biederman, Simo Sorce,
jlayton, Carlos O'Donell, Linux API, Linux Containers,
Linux Kernel, Eric Paris, David Howells, Linux Audit, Al Viro,
Andy Lutomirski, Linux Network Development, Linux FS Devel,
cgroups, Serge E. Hallyn, trondmy
In-Reply-To: <18cb69a5-f998-0e6e-85df-7f4b9b768a6f@schaufler-ca.com>
On Thu, Oct 19, 2017 at 9:32 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/18/2017 5:05 PM, Richard Guy Briggs wrote:
>> On 2017-10-17 01:10, Casey Schaufler wrote:
>>> On 10/16/2017 5:33 PM, Richard Guy Briggs wrote:
>>>> On 2017-10-12 16:33, Casey Schaufler wrote:
>>>>> On 10/12/2017 7:14 AM, Richard Guy Briggs wrote:
>>>>>> Containers are a userspace concept. The kernel knows nothing of them.
>>>>>>
>>>>>> The Linux audit system needs a way to be able to track the container
>>>>>> provenance of events and actions. Audit needs the kernel's help to do
>>>>>> this.
>>>>>>
>>>>>> Since the concept of a container is entirely a userspace concept, a
>>>>>> registration from the userspace container orchestration system initiates
>>>>>> this. This will define a point in time and a set of resources
>>>>>> associated with a particular container with an audit container ID.
>>>>>>
>>>>>> The registration is a pseudo filesystem (proc, since PID tree already
>>>>>> exists) write of a u8[16] UUID representing the container ID to a file
>>>>>> representing a process that will become the first process in a new
>>>>>> container. This write might place restrictions on mount namespaces
>>>>>> required to define a container, or at least careful checking of
>>>>>> namespaces in the kernel to verify permissions of the orchestrator so it
>>>>>> can't change its own container ID. A bind mount of nsfs may be
>>>>>> necessary in the container orchestrator's mntNS.
>>>>>> Note: Use a 128-bit scalar rather than a string to make compares faster
>>>>>> and simpler.
>>>>>>
>>>>>> Require a new CAP_CONTAINER_ADMIN to be able to carry out the
>>>>>> registration.
>>>>> Hang on. If containers are a user space concept, how can
>>>>> you want CAP_CONTAINER_ANYTHING? If there's not such thing as
>>>>> a container, how can you be asking for a capability to manage
>>>>> them?
>>>> There is such a thing, but the kernel doesn't know about it yet.
>>> Then how can it be the kernel's place to control access to a
>>> container resource, that is, the containerID.
>> Ok, let me try to address your objections.
>>
>> The kernel can know enough that if it is already set to not allow it to
>> be set again. Or if the user doesn't have permission to set it that the
>> user be denied this action. How is this different from loginuid and
>> sessionid?
>>>> This
>>>> same situation exists for loginuid and sessionid which are userspace
>>>> concepts that the kernel tracks for the convenience of userspace.
>>> Ah, no. Loginuid identifies a user, which is a kernel concept in
>>> that a user is defined by the uid.
>> This simple explanation doesn't help me. What makes that a kernel
>> concept? The fact that it is stored and compared in more than one
>> place?
>>
>>> The session ID has well defined kernel semantics. You're trying to say
>>> that the containerID is an opaque value that is meaningless to the
>>> kernel, but you still want the kernel to protect it. How can the
>>> kernel know if it is protecting it correctly?
>> How so? A userspace process triggers this. Does the kernel know what
>> these values mean? Does it do anything with them other than report
>> them or allow audit to filter them? It is given some instructions on
>> how to treat it.
>>
>> This is what we're trying to do with the containerID.
>>
>>>> As
>>>> for its name, I'm not particularly picky, so if you don't like
>>>> CAP_CONTAINER_* then I'm fine with CAP_AUDIT_CONTAINERID. It really
>>>> needs to be distinct from CAP_AUDIT_WRITE and CAP_AUDIT_CONTROL since we
>>>> don't want to give the ability to set a containerID to any process that
>>>> is able to do audit logging (such as vsftpd) and similarly we don't want
>>>> to give the orchestrator the ability to control the setup of the audit
>>>> daemon.
>>> Sorry, but what aspect of the kernel security policy is this
>>> capability supposed to protect? That's what capabilities are
>>> for, not the undefined support of undefined user-space behavior.
>> Similarly, loginuids and sessionIDs are only used for audit tracking and
>> filtering.
>
> Tell me again why you're not reusing either of these?
Ah, granularity arguments, welcome back old friend :)
Once again, we're still trying to sort all this out so I reserve the
right to change my mind, but my current thinking is as follows ...
CAP_AUDIT_WRITE exists to control which applications can submit
userspace generated audit records to the kernel, CAP_AUDIT_CONTROL
exists to control which applications can manage the in-kernel audit
configuration (e.g. filter rules) and the current task's loginuid
value. Reusing CAP_AUDIT_WRITE here would allow any application that
can submit userspace audit records the ability to change the audit
container ID; this would be bad, we don't allow CAP_AUDIT_WRITE to
change the loginuid, it would be even worse to allow it to change the
audit container ID. Reusing CAP_AUDIT_CONTROL is less worse than than
CAP_AUDIT_WRITE, but it gets sticky once we get to the part where we
want to auditd instances in containers, complete with their own
queues, filtering rules, etc.. Perhaps we could use CAP_AUDIT_CONTROL
to guard the audit container ID value, but we would always want to do
that check in the init userns in order to prevent container bound
processes from manipulating their own audit container ID.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Alexander Duyck @ 2017-10-19 15:53 UTC (permalink / raw)
To: Pavlos Parissis
Cc: Paweł Staszewski, Anders K. Pedersen | Cohaesio,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
alexander.h.duyck@intel.com
In-Reply-To: <CABOTfnOtw=XdhPL4n1JM16CS02ekoy6EPugOvtSbO6LGp2XOXA@mail.gmail.com>
On Thu, Oct 19, 2017 at 4:41 AM, Pavlos Parissis
<pavlos.parissis@gmail.com> wrote:
> On 19 October 2017 at 01:40, Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>>
>>
>> W dniu 2017-10-19 o 01:29, Alexander Duyck pisze:
>>
>>> On Mon, Oct 16, 2017 at 10:51 PM, Vitezslav Samel <vitezslav@samel.cz>
>>> wrote:
>>>>
>>>> On Tue, Oct 17, 2017 at 01:34:29AM +0200, Paweł Staszewski wrote:
>>>>>
>>>>> W dniu 2017-10-16 o 18:26, Paweł Staszewski pisze:
>>>>>>
>>>>>> W dniu 2017-10-16 o 13:20, Pavlos Parissis pisze:
>>>>>>>
>>>>>>> On 15/10/2017 02:58 πμ, Alexander Duyck wrote:
>>>>>>>>
>>>>>>>> Hi Pawel,
>>>>>>>>
>>>>>>>> To clarify is that Dave Miller's tree or Linus's that you are talking
>>>>>>>> about? If it is Dave's tree how long ago was it you pulled it since I
>>>>>>>> think the fix was just pushed by Jeff Kirsher a few days ago.
>>>>>>>>
>>>>>>>> The issue should be fixed in the following commit:
>>>>>>>>
>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/drivers/net/ethernet/intel/i40e/i40e_txrx.c?id=2b9478ffc550f17c6cd8c69057234e91150f5972
>>>>>>>
>>>>>>> Do you know when it is going to be available on net-next and
>>>>>>> linux-stable repos?
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Pavlos
>>>>>>>
>>>>>>>
>>>>>> I will make some tests today night with "net" git tree where this patch
>>>>>> is included.
>>>>>> Starting from 0:00 CET
>>>>>> :)
>>>>>>
>>>>>>
>>>>> Upgraded and looks like problem is not solved with that patch
>>>>> Currently running system with
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/
>>>>> kernel
>>>>>
>>>>> Still about 0.5GB of memory is leaking somewhere
>>>>>
>>>>> Also can confirm that the latest kernel where memory is not leaking
>>>>> (with
>>>>> use i40e driver intel 710 cards) is 4.11.12
>>>>> With kernel 4.11.12 - after hour no change in memory usage.
>>>>>
>>>>> also checked that with ixgbe instead of i40e with same net.git kernel
>>>>> there
>>>>> is no memleak - after hour same memory usage - so for 100% this is i40e
>>>>> driver problem.
>>>>
>>>> I have (probably) the same problem here but with X520 cards: booting
>>>> 4.12.x gives me oops after circa 20 minutes of our workload. Booting
>>>> 4.9.y is OK. This machine is in production so any testing is very
>>>> limited.
>>>>
>>>> Machine was stable for >2 months (on the desk before got to
>>>> production) with 4.12.8 but with no traffic on X520 cards.
>>>>
>>>> Cheers,
>>>>
>>>> Vita
>>>
>>> Sorry but it can't be the same issue since we are discussing a
>>> different driver (i40e) running different hardware (X710 or XL170).
>>> You might want to start a new thread for your issue, and/or if
>>> possible file a bug on e1000.sf.net.
>>>
>>> Thanks.
>>>
>>> - Alex
>>>
>> sorry but bugs reported on e1000.sf.net are delayed - some after about 6 or
>> more months - when i reported first bug there iv got reply after a year
>> about no activity :):) haha - and reported there bug is still actrive :)
>> better for me is now to change nics (for sure cheaper from the perspective
>> of clients :) ) to mellanox or just to replace and use ixgbe - that have no
>> this bug (mellanox and ixgbe have no such bug - have many servers with them
>> with same conf - and only one with i40e where is same conf and memleak)
>>
>> If nobody from Intel wants to reproduce this - qool - this is not my problem
>> but intels :) - there is now many good nics to use - like mellanox or just
>> stick with many 10G based on ixgbe that is really good driver - but really ?
>> intel guys have no XL710 cards ? i dont want to buy another buggy cards to
>> do only kernel bisects .... sorry ....
>> To do good bisects with this bug You need to spend maybee 200/300 bisects -
>> and to confirm each - You need maybee 30minutes so count how much time You
>> need - more that 100 cards in price from mellanox maybee :)
>>
>
> I have similar issues with you in regards to the stability of i40e
> driver. I will need to open another thread about them, but I would
> like to mention that you are not the only one who suffers from
> problems related to i40e driver. In my case I can't simply change
> NICs..so it is even worse.
>
> Cheers,
> Pavlos
Hi Pavlos,
If you want feel free to Cc either my gmail or my intel.com email
address when you start the new thread, and I can work with you to try
to resolve the issues you are experiencing.
I'm just wanting to split up the unrelated issues into separate
threads as it is easier to track them as single threads. It makes it
much easier to figure out when an actual issue such as the original
memory leak was resolved versus trying to work multiple issues on the
same thread which makes things confusing as you end up losing track of
what the issue being resolved actually is, and it makes it confusing
for people who are reviewing the mailing list for issues similar to
what they are experiencing.
Thanks for your input, and I look forward to working with you to
resolve the issue you are experiencing.
- Alex
^ permalink raw reply
* [PATCH net-next 0/2] bridge: make setlink/dellink notifications more accurate
From: Nikolay Aleksandrov @ 2017-10-19 15:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsa, mrv, stephen, bridge, Nikolay Aleksandrov
Hi,
Before this set the bridge would generate a notification on vlan add or del
even if they didn't actually do any changes, which confuses listeners and
is generally not preferred. We could also lose notifications on actual
changes if one adds a range of vlans and there's an error in the middle.
The problem with just breaking and returning an error is that we could
break existing user-space scripts which rely on the vlan delete to clear
all existing entries in the specified range and ignore the non-existing
errors (typically used to clear the current vlan config).
So in order to make the notifications more accurate while keeping backwards
compatibility we add a boolean that tracks if anything actually changed
during the config calls.
The vlan add is more difficult to fix because it always returns 0 even if
nothing changed, so we use EEXIST to signal that and in order not to break
overlapping vlan range add or script return expectations we clear it on
return, the functions used by vlan add are not expected to return EEXIST.
Thanks,
Nik
Nikolay Aleksandrov (2):
bridge: netlink: make setlink/dellink notifications more accurate
bridge: vlan: return EEXIST on add if nothing changed
net/bridge/br_netlink.c | 49 ++++++++++++++++++++++++++----------------
net/bridge/br_netlink_tunnel.c | 14 +++++++-----
net/bridge/br_private_tunnel.h | 3 ++-
net/bridge/br_vlan.c | 38 +++++++++++++++++++++-----------
4 files changed, 68 insertions(+), 36 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH net-next 1/2] bridge: netlink: make setlink/dellink notifications more accurate
From: Nikolay Aleksandrov @ 2017-10-19 15:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsa, mrv, stephen, bridge, Nikolay Aleksandrov
In-Reply-To: <1508428499-32355-1-git-send-email-nikolay@cumulusnetworks.com>
Before this patch we had cases that either sent notifications when there
were in fact no changes (e.g. non-existent vlan delete) or didn't send
notifications when there were changes (e.g. vlan add range with an error in
the middle, port flags change + vlan update error). This patch sends down
a boolean to the functions setlink/dellink use and if there is even a
single configuration change (port flag, vlan add/del, port state) then
we always send a notification. This is all done to keep backwards
compatibility with the opportunistic vlan delete, where one could
specify a vlan range that has missing vlans inside and still everything
in that range will be cleared, this is mostly used to clear the whole
vlan config with a single call, i.e. range 1-4094.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink.c | 44 +++++++++++++++++++++++++-----------------
net/bridge/br_netlink_tunnel.c | 14 +++++++++-----
net/bridge/br_private_tunnel.h | 3 ++-
3 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index f0e82682e071..e8d74a3f44f7 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -506,7 +506,7 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
}
static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
- int cmd, struct bridge_vlan_info *vinfo)
+ int cmd, struct bridge_vlan_info *vinfo, bool *changed)
{
int err = 0;
@@ -517,21 +517,24 @@ static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
* per-VLAN entry as well
*/
err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
- if (err)
- break;
} else {
vinfo->flags |= BRIDGE_VLAN_INFO_BRENTRY;
err = br_vlan_add(br, vinfo->vid, vinfo->flags);
}
+ if (!err)
+ *changed = true;
break;
case RTM_DELLINK:
if (p) {
- nbp_vlan_delete(p, vinfo->vid);
- if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
- br_vlan_delete(p->br, vinfo->vid);
- } else {
- br_vlan_delete(br, vinfo->vid);
+ if (!nbp_vlan_delete(p, vinfo->vid))
+ *changed = true;
+
+ if ((vinfo->flags & BRIDGE_VLAN_INFO_MASTER) &&
+ !br_vlan_delete(p->br, vinfo->vid))
+ *changed = true;
+ } else if (!br_vlan_delete(br, vinfo->vid)) {
+ *changed = true;
}
break;
}
@@ -542,7 +545,8 @@ static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
static int br_process_vlan_info(struct net_bridge *br,
struct net_bridge_port *p, int cmd,
struct bridge_vlan_info *vinfo_curr,
- struct bridge_vlan_info **vinfo_last)
+ struct bridge_vlan_info **vinfo_last,
+ bool *changed)
{
if (!vinfo_curr->vid || vinfo_curr->vid >= VLAN_VID_MASK)
return -EINVAL;
@@ -572,7 +576,7 @@ static int br_process_vlan_info(struct net_bridge *br,
sizeof(struct bridge_vlan_info));
for (v = (*vinfo_last)->vid; v <= vinfo_curr->vid; v++) {
tmp_vinfo.vid = v;
- err = br_vlan_info(br, p, cmd, &tmp_vinfo);
+ err = br_vlan_info(br, p, cmd, &tmp_vinfo, changed);
if (err)
break;
}
@@ -581,13 +585,13 @@ static int br_process_vlan_info(struct net_bridge *br,
return 0;
}
- return br_vlan_info(br, p, cmd, vinfo_curr);
+ return br_vlan_info(br, p, cmd, vinfo_curr, changed);
}
static int br_afspec(struct net_bridge *br,
struct net_bridge_port *p,
struct nlattr *af_spec,
- int cmd)
+ int cmd, bool *changed)
{
struct bridge_vlan_info *vinfo_curr = NULL;
struct bridge_vlan_info *vinfo_last = NULL;
@@ -607,7 +611,8 @@ static int br_afspec(struct net_bridge *br,
return err;
err = br_process_vlan_tunnel_info(br, p, cmd,
&tinfo_curr,
- &tinfo_last);
+ &tinfo_last,
+ changed);
if (err)
return err;
break;
@@ -616,7 +621,7 @@ static int br_afspec(struct net_bridge *br,
return -EINVAL;
vinfo_curr = nla_data(attr);
err = br_process_vlan_info(br, p, cmd, vinfo_curr,
- &vinfo_last);
+ &vinfo_last, changed);
if (err)
return err;
break;
@@ -804,6 +809,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
struct nlattr *afspec;
struct net_bridge_port *p;
struct nlattr *tb[IFLA_BRPORT_MAX + 1];
+ bool changed = false;
int err = 0;
protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_PROTINFO);
@@ -839,14 +845,15 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
}
if (err)
goto out;
+ changed = true;
}
if (afspec) {
err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
- afspec, RTM_SETLINK);
+ afspec, RTM_SETLINK, &changed);
}
- if (err == 0)
+ if (changed)
br_ifinfo_notify(RTM_NEWLINK, p);
out:
return err;
@@ -857,6 +864,7 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
{
struct nlattr *afspec;
struct net_bridge_port *p;
+ bool changed = false;
int err = 0;
afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
@@ -869,8 +877,8 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
return -EINVAL;
err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
- afspec, RTM_DELLINK);
- if (err == 0)
+ afspec, RTM_DELLINK, &changed);
+ if (changed)
/* Send RTM_NEWLINK because userspace
* expects RTM_NEWLINK for vlan dels
*/
diff --git a/net/bridge/br_netlink_tunnel.c b/net/bridge/br_netlink_tunnel.c
index 3712c7f0e00c..da8cb99fd259 100644
--- a/net/bridge/br_netlink_tunnel.c
+++ b/net/bridge/br_netlink_tunnel.c
@@ -198,7 +198,7 @@ static const struct nla_policy vlan_tunnel_policy[IFLA_BRIDGE_VLAN_TUNNEL_MAX +
};
static int br_vlan_tunnel_info(struct net_bridge_port *p, int cmd,
- u16 vid, u32 tun_id)
+ u16 vid, u32 tun_id, bool *changed)
{
int err = 0;
@@ -208,9 +208,12 @@ static int br_vlan_tunnel_info(struct net_bridge_port *p, int cmd,
switch (cmd) {
case RTM_SETLINK:
err = nbp_vlan_tunnel_info_add(p, vid, tun_id);
+ if (!err)
+ *changed = true;
break;
case RTM_DELLINK:
- nbp_vlan_tunnel_info_delete(p, vid);
+ if (!nbp_vlan_tunnel_info_delete(p, vid))
+ *changed = true;
break;
}
@@ -254,7 +257,8 @@ int br_parse_vlan_tunnel_info(struct nlattr *attr,
int br_process_vlan_tunnel_info(struct net_bridge *br,
struct net_bridge_port *p, int cmd,
struct vtunnel_info *tinfo_curr,
- struct vtunnel_info *tinfo_last)
+ struct vtunnel_info *tinfo_last,
+ bool *changed)
{
int err;
@@ -272,7 +276,7 @@ int br_process_vlan_tunnel_info(struct net_bridge *br,
return -EINVAL;
t = tinfo_last->tunid;
for (v = tinfo_last->vid; v <= tinfo_curr->vid; v++) {
- err = br_vlan_tunnel_info(p, cmd, v, t);
+ err = br_vlan_tunnel_info(p, cmd, v, t, changed);
if (err)
return err;
t++;
@@ -283,7 +287,7 @@ int br_process_vlan_tunnel_info(struct net_bridge *br,
if (tinfo_last->flags)
return -EINVAL;
err = br_vlan_tunnel_info(p, cmd, tinfo_curr->vid,
- tinfo_curr->tunid);
+ tinfo_curr->tunid, changed);
if (err)
return err;
memset(tinfo_last, 0, sizeof(struct vtunnel_info));
diff --git a/net/bridge/br_private_tunnel.h b/net/bridge/br_private_tunnel.h
index 4a447a378ab3..a259471bfd78 100644
--- a/net/bridge/br_private_tunnel.h
+++ b/net/bridge/br_private_tunnel.h
@@ -26,7 +26,8 @@ int br_process_vlan_tunnel_info(struct net_bridge *br,
struct net_bridge_port *p,
int cmd,
struct vtunnel_info *tinfo_curr,
- struct vtunnel_info *tinfo_last);
+ struct vtunnel_info *tinfo_last,
+ bool *changed);
int br_get_vlan_tunnel_info_size(struct net_bridge_vlan_group *vg);
int br_fill_vlan_tunnel_info(struct sk_buff *skb,
struct net_bridge_vlan_group *vg);
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/2] bridge: vlan: return EEXIST on add if nothing changed
From: Nikolay Aleksandrov @ 2017-10-19 15:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsa, mrv, stephen, bridge, Nikolay Aleksandrov
In-Reply-To: <1508428499-32355-1-git-send-email-nikolay@cumulusnetworks.com>
Before this patch there was no way to tell if the vlan add operation
actually changed anything, thus we would always generate a notification
on adds. Let's make the notifications more precise and generate them
only if anything changed, so use EEXIST error to signal that the vlan
was not updated in any way. We just need to be careful about a few
places that could re-add the same vlan with the same flags not to return
any errors on EEXIST.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink.c | 5 +++++
net/bridge/br_vlan.c | 38 ++++++++++++++++++++++++++------------
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e8d74a3f44f7..80d9334a4b46 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -523,6 +523,11 @@ static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
}
if (!err)
*changed = true;
+ else if (err == -EEXIST)
+ /* nothing changed, return 0 for overlapping range add
+ * and compatibility reasons
+ */
+ err = 0;
break;
case RTM_DELLINK:
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 233a30040c91..e021a80eb8e9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -32,27 +32,34 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
return rhashtable_lookup_fast(tbl, &vid, br_vlan_rht_params);
}
-static void __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+static bool __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
{
if (vg->pvid == vid)
- return;
+ return false;
smp_wmb();
vg->pvid = vid;
+
+ return true;
}
-static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+static bool __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
{
if (vg->pvid != vid)
- return;
+ return false;
smp_wmb();
vg->pvid = 0;
+
+ return true;
}
-static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
+/* return true if anything changed, false otherwise */
+static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
{
struct net_bridge_vlan_group *vg;
+ u16 old_flags = v->flags;
+ bool ret;
if (br_vlan_is_master(v))
vg = br_vlan_group(v->br);
@@ -60,14 +67,16 @@ static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
vg = nbp_vlan_group(v->port);
if (flags & BRIDGE_VLAN_INFO_PVID)
- __vlan_add_pvid(vg, v->vid);
+ ret = __vlan_add_pvid(vg, v->vid);
else
- __vlan_delete_pvid(vg, v->vid);
+ ret = __vlan_delete_pvid(vg, v->vid);
if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
else
v->flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
+
+ return ret || !!(old_flags ^ v->flags);
}
static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
@@ -234,7 +243,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
if (flags & BRIDGE_VLAN_INFO_MASTER) {
err = br_vlan_add(br, v->vid, flags |
BRIDGE_VLAN_INFO_BRENTRY);
- if (err)
+ if (err && err != -EEXIST)
goto out_filt;
}
@@ -562,6 +571,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
vg = br_vlan_group(br);
vlan = br_vlan_find(vg, vid);
if (vlan) {
+ ret = -EEXIST;
if (!br_vlan_is_brentry(vlan)) {
/* Trying to change flags of non-existent bridge vlan */
if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
@@ -577,8 +587,10 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
vg->num_vlans++;
}
- __vlan_add_flags(vlan, flags);
- return 0;
+ if (__vlan_add_flags(vlan, flags))
+ ret = 0;
+
+ return ret;
}
vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
@@ -870,7 +882,7 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
err = nbp_vlan_add(p, pvid,
BRIDGE_VLAN_INFO_PVID |
BRIDGE_VLAN_INFO_UNTAGGED);
- if (err)
+ if (err && err != -EEXIST)
goto err_port;
nbp_vlan_delete(p, old_pvid);
set_bit(p->port_no, changed);
@@ -1037,7 +1049,9 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
ret = switchdev_port_obj_add(port->dev, &v.obj);
if (ret && ret != -EOPNOTSUPP)
return ret;
- __vlan_add_flags(vlan, flags);
+ if (!__vlan_add_flags(vlan, flags))
+ return -EEXIST;
+
return 0;
}
--
2.1.4
^ permalink raw reply related
* [net PATCH] bpf: devmap fix arithmetic overflow in bitmap_size calculation
From: John Fastabend @ 2017-10-19 16:03 UTC (permalink / raw)
To: richard, alexei.starovoitov, davem; +Cc: netdev, borkmann
An integer overflow is possible in dev_map_bitmap_size() when
calculating the BITS_TO_LONG logic which becomes, after macro
replacement,
(((n) + (d) - 1)/ (d))
where 'n' is a __u32 and 'd' is (8 * sizeof(long)). To avoid
overflow cast to u64 before arithmetic.
Reported-by: Richard Weinberger <richard@nod.at>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/devmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 7d9f32f..6d3ec97 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -69,7 +69,7 @@ static LIST_HEAD(dev_map_list);
static u64 dev_map_bitmap_size(const union bpf_attr *attr)
{
- return BITS_TO_LONGS(attr->max_entries) * sizeof(unsigned long);
+ return BITS_TO_LONGS((u64) attr->max_entries) * sizeof(unsigned long);
}
static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
^ permalink raw reply related
* Re: [PATCH net-next 0/2] bridge: make setlink/dellink notifications more accurate
From: Nikolay Aleksandrov @ 2017-10-19 16:10 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsa, mrv, stephen, bridge
In-Reply-To: <1508428499-32355-1-git-send-email-nikolay@cumulusnetworks.com>
On 19/10/17 18:54, Nikolay Aleksandrov wrote:
> Hi,
> Before this set the bridge would generate a notification on vlan add or del
> even if they didn't actually do any changes, which confuses listeners and
> is generally not preferred. We could also lose notifications on actual
> changes if one adds a range of vlans and there's an error in the middle.
> The problem with just breaking and returning an error is that we could
> break existing user-space scripts which rely on the vlan delete to clear
> all existing entries in the specified range and ignore the non-existing
> errors (typically used to clear the current vlan config).
> So in order to make the notifications more accurate while keeping backwards
> compatibility we add a boolean that tracks if anything actually changed
> during the config calls.
>
> The vlan add is more difficult to fix because it always returns 0 even if
> nothing changed, so we use EEXIST to signal that and in order not to break
> overlapping vlan range add or script return expectations we clear it on
> return, the functions used by vlan add are not expected to return EEXIST.
>
> Thanks,
> Nik
>
> Nikolay Aleksandrov (2):
> bridge: netlink: make setlink/dellink notifications more accurate
> bridge: vlan: return EEXIST on add if nothing changed
>
> net/bridge/br_netlink.c | 49 ++++++++++++++++++++++++++----------------
> net/bridge/br_netlink_tunnel.c | 14 +++++++-----
> net/bridge/br_private_tunnel.h | 3 ++-
> net/bridge/br_vlan.c | 38 +++++++++++++++++++++-----------
> 4 files changed, 68 insertions(+), 36 deletions(-)
>
Self-NAK
Dave, please ignore this set and apologies for the noise.
I'd actually prefer to send "changed" down to vlan_add and vlan_del
to be set there instead of overloading EEXIST like that. It will be safer.
I'll wait for some comments about the rest of the change and will send
a v2.
Thanks,
Nik
^ permalink raw reply
* Re: [kernel-hardening] [PATCH 0/2] capability controlled user-namespaces
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-10-19 16:15 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Mahesh Bandewar, LKML, Netdev, Kernel-hardening, Linux API,
Kees Cook, Eric W . Biederman, Eric Dumazet, David Miller
In-Reply-To: <CAF2d9jjc6z_cO7co_C7L7xPujqA2npa2nXpUHZbE-Le6BSGFNg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Oct 2, 2017 at 11:12 AM, Mahesh Bandewar (महेश बंडेवार)
<maheshb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, Oct 2, 2017 at 10:14 AM, Serge E. Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> wrote:
>> Quoting Mahesh Bandewar (mahesh-bmGAjcP2qsnk1uMJSBkQmQ@public.gmane.org):
>>> From: Mahesh Bandewar <maheshb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>>
>>> [Same as the previous RFC series sent on 9/21]
>>>
>>> TL;DR version
>>> -------------
>>> Creating a sandbox environment with namespaces is challenging
>>> considering what these sandboxed processes can engage into. e.g.
>>> CVE-2017-6074, CVE-2017-7184, CVE-2017-7308 etc. just to name few.
>>> Current form of user-namespaces, however, if changed a bit can allow
>>> us to create a sandbox environment without locking down user-
>>> namespaces.
>>>
>>> Detailed version
>>> ----------------
>>
>> Hi,
>>
>> still struggling with how I feel about the idea in general.
>>
>> So is the intent mainly that if/when there comes an 0-day which allows
>> users with CAP_NET_ADMIN in any namespace to gain privilege on the host,
>> then this can be used as a stop-gap measure until there is a proper fix?
>>
> Thank for looking at this Serge.
>
> Yes, but at the same time it's not just limited to NET_ADMIN but could
> be any of the current capabilities.
>
>> Otherwise, do you have any guidance for how people should use this?
>>
>> IMO it should be heavily discouraged to use this tool as a regular
>> day to day configuration, as I'm not sure there is any "educated"
>> decision to be made, even by those who are in the know, about what
>> to put in this set.
>>
> I think that really depends on the environment. e.g. in certain
> sandboxes third-part / semi-trusted workload is executed where network
> resource is not used. In that environment I can easily take off
> NET_ADMIN and NET_RAW without affecting anything there. At the same
> time I wont have to worry about 0-day related to these two
> capabilities. I would say the Admins at these places are in the best
> place to decide what they can take-off safely and what they cannot.
> Even if they decide not to take-off anything, having a tool at hand to
> gain control is important when the next 0-day strikes us that can be
> exploited using any of the currently used capabilities.
>
> However, you are absolutely right in terms of using it as a stop-gap
> measure to protect environment until it's fixed and the capability in
> question can not be safely taken off permanently without hampering
> operations.
>
> thanks,
> --mahesh..
>
> [...]
friendly ping.
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: Eric W. Biederman @ 2017-10-19 16:25 UTC (permalink / raw)
To: Paul Moore
Cc: Aleksa Sarai, James Bottomley, cgroups-u79uwXL29TY76Z2rM5mHXA,
mszeredi-H+wXaHxf7aLQT0dZR+AlfA, Andy Lutomirski,
jlayton-H+wXaHxf7aLQT0dZR+AlfA, Carlos O'Donell, API,
Linux Containers, Linux Kernel, Viro, David Howells,
Linux FS Devel, linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Simo Sorce,
Development, Casey Schaufler, Eric Paris, Steve Grubb,
trondmy-7I+n7zu2hftEKMMhf/gKZA
In-Reply-To: <CAHC9VhTYF-MJm3ejWXE1H-eeXKaNBkeWKwdiKdj093xATYn7nQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Paul Moore <paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org> writes:
> On Wed, Oct 18, 2017 at 8:43 PM, Eric W. Biederman
> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>> Aleksa Sarai <asarai-l3A5Bk7waGM@public.gmane.org> writes:
>>>>> The security implications are that anything that can change the label
>>>>> could also hide itself and its doings from the audit system and thus
>>>>> would be used as a means to evade detection. I actually think this
>>>>> means the label should be write once (once you've set it, you can't
>>>>> change it) ...
>>>>
>>>> Richard and I have talked about a write once approach, but the
>>>> thinking was that you may want to allow a nested container
>>>> orchestrator (Why? I don't know, but people always want to do the
>>>> craziest things.) and a write-once policy makes that impossible. If
>>>> we punt on the nested orchestrator, I believe we can seriously think
>>>> about a write-once policy to simplify things.
>>>
>>> Nested containers are a very widely used use-case (see LXC system containers,
>>> inside of which people run other container runtimes). So I would definitely
>>> consider it something that "needs to be supported in some way". While the LXC
>>> guys might be a *tad* crazy, the use-case isn't. :P
>
> No worries, we're all a little crazy in our own special ways ;)
>
> Kidding aside, thanks for explaining the use case.
>
>> Of course some of that gets to running auditd inside a container which
>> we don't have yet either.
>>
>> So I think to start it is perfectly fine to figure out the non-nested
>> case first and what makes sense there. Then to sort out the nested
>> container case.
>>
>> The solution might be that a process gets at most one id per ``audit
>> namespace''.
>
> In an attempt to stay on-topic, let's try to stick with "audit
> container ID" or "container ID" if you must. I really want to avoid
> the term "audit namespace" simply because the term "namespace" implies
> some things which we aren't planning on doing.
This is 100% on topic. I am saying that unless we are planing to have
auditd running in a container with it's own set of rules you probably
don't care about nested containers. Last time I heard a discussion
about that the term in use was audit namespace. So I was referring to
that support when I said audit namespace, even if the end result only
loosely fits the term namespace.
I could be wrong of course. I don't fully understand what is driving
the desire to connect audit and containers. But my naive guess is that
one from an audit perspective you don't care about nested containers
unless there is also a nested auditd who is looking at it from a nested
perspective.
So far we have established with the term container that we are talking
about a running instance of processes, not a filesystem instance that
Docker and friends ship around. Beyond that I am not certain what you
care about.
Eric
^ permalink raw reply
* [PATCH net] sock: correct sk_wmem_queued accounting on efault in tcp zerocopy
From: Willem de Bruijn @ 2017-10-19 16:40 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Syzkaller hits WARN_ON(sk->sk_wmem_queued) in sk_stream_kill_queues
after triggering an EFAULT in __zerocopy_sg_from_iter.
On this error, skb_zerocopy_stream_iter resets the skb to its state
before the operation with __pskb_trim. It cannot kfree_skb like
datagram callers, as the skb may have data from a previous send call.
__pskb_trim calls skb_condense for unowned skbs, which adjusts their
truesize. These tcp skbuffs are owned and their truesize must add up
to sk_wmem_queued. But they match because their skb->sk is NULL until
tcp_transmit_skb.
Temporarily set skb->sk when calling __pskb_trim to signal that the
skbuffs are owned and avoid the skb_condense path.
Fixes: 52267790ef52 ("sock: add MSG_ZEROCOPY")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e62476beee95..24656076906d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1124,9 +1124,13 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
err = __zerocopy_sg_from_iter(sk, skb, &msg->msg_iter, len);
if (err == -EFAULT || (err == -EMSGSIZE && skb->len == orig_len)) {
+ struct sock *save_sk = skb->sk;
+
/* Streams do not free skb on error. Reset to prev state. */
msg->msg_iter = orig_iter;
+ skb->sk = sk;
___pskb_trim(skb, orig_len);
+ skb->sk = save_sk;
return err;
}
--
2.15.0.rc1.287.g2b38de12cc-goog
^ permalink raw reply related
* Re: [PATCH net] sock: correct sk_wmem_queued accounting on efault in tcp zerocopy
From: Eric Dumazet @ 2017-10-19 16:45 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, David Miller, Willem de Bruijn
In-Reply-To: <20171019164039.20927-1-willemdebruijn.kernel@gmail.com>
On Thu, Oct 19, 2017 at 9:40 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Syzkaller hits WARN_ON(sk->sk_wmem_queued) in sk_stream_kill_queues
> after triggering an EFAULT in __zerocopy_sg_from_iter.
>
> On this error, skb_zerocopy_stream_iter resets the skb to its state
> before the operation with __pskb_trim. It cannot kfree_skb like
> datagram callers, as the skb may have data from a previous send call.
>
> __pskb_trim calls skb_condense for unowned skbs, which adjusts their
> truesize. These tcp skbuffs are owned and their truesize must add up
> to sk_wmem_queued. But they match because their skb->sk is NULL until
> tcp_transmit_skb.
>
> Temporarily set skb->sk when calling __pskb_trim to signal that the
> skbuffs are owned and avoid the skb_condense path.
>
> Fixes: 52267790ef52 ("sock: add MSG_ZEROCOPY")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks a lot Willem.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] net: dsa: lan9303: Add port_fast_age and port_fdb_dump methods
From: Egil Hjelmeland @ 2017-10-19 16:52 UTC (permalink / raw)
To: David Laight, 'Andrew Lunn', Vivien Didelot
Cc: f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <35825cd2-0b47-3f8a-813e-1e7db2f7dcde@egil-hjelmeland.no>
Den 19. okt. 2017 17:42, skrev Egil Hjelmeland:
> On 19. okt. 2017 17:15, David Laight wrote:
>> From: Andrew Lunn
>>> Sent: 19 October 2017 15:15
>>>>> +/* Clear learned (non-static) entry on given port */
>>>>> +static void alr_loop_cb_del_port_learned(struct lan9303 *chip, u32
>>>>> dat0,
>>>>> + u32 dat1, int portmap, void *ctx)
>>>>> +{
>>>>> + int *port = ctx;
>>>>
>>>> You can get the value directly to make the line below more readable:
>>>>
>>>> int port = *(int *)ctx;
>>>
>>> You have to be a bit careful with this. You often see people
>>> submitting patches taking away casts for void * pointers.
>>> If they do that here, it should at least not compile...
>>>
>>> So maybe do it in two steps?
>>>
>>> int * pport = ctx;
>>> int port = *pport;
>>
>> IMHO it is best to define a struct for the 'ctx and then do:
>> ..., void *v_ctx)
>> {
>> foo_ctx *ctx = v_ctx;
>> int port = ctx->port;
>>
>> That stops anyone having to double-check that the *(int *)
>> is operating on a pointer to an integer of the correct size.
>>
>
> Does casting to a struct pointer require less manual double-check than
> to a int-pointer? In neither cases the compiler can protect us, IFAIK.
> But on the other hand, a the text "foo_ctx" can searched in the editor.
> So in that respect it can somewhat aid to the double-checking.
>
> So I can do that.
>
>
I understand now that the caller side (lan9303_port_fast_age) is
vulnerable. Say somebody has the idea to change the "port" param
of .port_fast_age from int to u8, then my code is a trap.
Thanks for the education.
Egil
^ permalink raw reply
* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Alexander Duyck @ 2017-10-19 17:10 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Pavlos Parissis, Anders K. Pedersen | Cohaesio,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
alexander.h.duyck@intel.com
In-Reply-To: <d4d1c562-e976-1894-e36c-709fd319dfb2@itcare.pl>
On Wed, Oct 18, 2017 at 4:51 PM, Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>
>
> W dniu 2017-10-19 o 01:37, Alexander Duyck pisze:
>
>> On Wed, Oct 18, 2017 at 4:22 PM, Paweł Staszewski <pstaszewski@itcare.pl>
>> wrote:
>>>
>>>
>>> W dniu 2017-10-19 o 00:58, Paweł Staszewski pisze:
<snip>
>>> Change rx-usecs 16 tx usecs 16
>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>> enp3s0f3'
>>> for i in $ifc
>>> do
>>> ip link set up dev $i
>>> ethtool -A $i autoneg off rx off tx off
>>> ethtool -G $i rx 2048 tx 2048
>>> ip link set $i txqueuelen 1000
>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 16
>>> tx-usecs
>>> 16
>>> ethtool -L $i combined 6
>>> ethtool -K $i ntuple on
>>> ethtool -K $i gro on
>>> ethtool -K $i tso on
>>> done
>>>
>>> MEMLEAK: 0-2MB/s with some recycles
>>> 0 MB/10sec
>>> 0 MB/10sec
>>> 0 MB/10sec
>>> 0 MB/10sec
>>> 0 MB/10sec
>>> 0 MB/10sec
>>> 1 MB/10sec
>>> 0 MB/10sec
>>> 2 MB/10sec
>>> 0 MB/10sec
>>> 2 MB/10sec
>>> -1 MB/10sec
>>> 0 MB/10sec
>>> 2 MB/10sec
>>> 0 MB/10sec
>>> 2 MB/10sec
>>> -1 MB/10sec
>>> 1 MB/10sec
>>
>> This data doesn't tell me much of anything and isn't what I asked for.
>> I don't see how the interrupt throttling rate would be associated with
>> your memory leak other than possibly rate limiting it by rate limiting
>> the traffic itself. Is there something that gave you the impression
>> that interrupt rate was somehow involved?
>
> more interrupts more leak
Right, but this isn't really any new information. More general
activity equals more memory leak, we already knew that.
When debugging, what is useful is to isolate a problem down to a
subset of the original problem. So for example, if you are seeing some
traffic pattern that makes it significantly worse that would be useful
info. Generally instead of trying to find cases that make the issue
less likely to happen anything that you can find that make it more
likely to happen would be useful.
Anything that makes the reproduction easier to get would be useful as
it makes the test for the issue being fixed that much easier since
there should be a start contrast for the driver with the issue versus
the driver without. A fast failure is always much easier to diagnose
than a lingering issue.
>>
>> When we last talked I had asked if you could do a git bisect to find
>> the memory leak and you said you would look into it. The most useful
>> way to solve this would be to do a git bisect between your current
>> kernel and the 4.11 kernel to find the point at which this started. If
>> we can do that then fixing this becomes much simpler as we just have
>> to fix the patch that introduced the issue.
>>
>> Also, I don't know it is you are using to determine that there is a
>> memory leak. What tool is it you are using to do the tracking? Is
>> there any specific form of traffic that is causing the leak? If you
>> can't perform the bisection, any information you could provide that
>> would allow me to do it would also be useful.
>
> simple script
>
> mem1=`free -m | grep Mem: | awk '{print $3}'`
> sleep 10
> mem2=`free -m | grep Mem: | awk '{print $3}'`
>
> num=$((mem2 - mem1))
> echo $num " MB/10sec"
>
>
> There is nothing more that gets mem
> there is only routed traffic from interface A to B
> nothings takes mem
> And memleaks only anchge when i change the rx/tx usecs for card
>
> What You need more ?
No more needed. This tells me what I need to be able to start a
reproduction setup.
> imagine this is not my only prblem but many - i just want to help i changed
> cards to i40e based only cause somebody rises a bug - and i want to use i40e
> in feature - dont need them now - but maybee it is good to help ppl to solwe
> some problems now if i can - before i will use this cards ?
Your attempt to help is appreciated, and I want to solve these issues as well.
> I try to use i40e before but there was bug covered by bug - and nobody from
> e1000.sf can help me they just reply after year and closing tickets with
> info about no activity but they have info in reported bugs ... soooo what is
> this ? support center ? for me no .
We don't ask you to be a support center, but we need to have either a
clear problem definition or a willingness to provide more information.
As you stated there are multiple issues with the driver, it isn't
perfect since it is developed by people and people make mistakes. We
need a very clear problem statement and reproduction steps when an
issue is reported so that we can first try to reproduce the issue, and
secondly try to verify that it has been resolved. When the problem
statement is vague, and you aren't willing to test fixes or providing
debugging information there isn't much we can do. As such we are much
more likely to close a bug ticket when we cannot reproduce the issue,
and you are not willing to work with us to test possible fixes for the
issue.
> If i want to help -= after a year response will be something like - "dont
> care now" - cause i'v used other hw or sme hacks to repair problem that
> should be sloved by intel
You are welcome to help. This thread started out focusing on the i40e
memory leak issue being reported by Anders, and I would appreciate it
if we could keep this focused on the i40e memory leak. Trying to
hijack the thread to address other issues that may have been reported
on e1000.sf.net, but closed for whatever reason isn't productive. We
need to focus on one issue per thread at a time, just complaining that
something is buggy isn't productive and doesn't solve any issues.
We have your definition for the problem you are seeing. We can work on
trying to reproduce the issue in our environment. Our internal
validation hasn't seen this issue so we likely have some sort of test
escape internally that we need to resolve in our validation
environment.
Anders has said he would be willing to work with us on getting a
bisection. That will help significantly for us to try to get to a root
cause for this issue. In addition once we have the root cause we can
also start sorting out why we didn't catch this in our own validation.
Your help on this issue has been appreciated, but if you aren't
willing to perform a bisection then there isn't any more we need from
you at this time. We will work internally and with Anders to get the
bisection data we need. We have no further need of information on the
issue at this time as we need to focus on test reproduction, and
determining the change that introduced this issue.
Thanks.
- Alex
^ permalink raw reply
* [PATCH net] net: bridge: fix returning of vlan range op errors
From: Nikolay Aleksandrov @ 2017-10-19 17:17 UTC (permalink / raw)
To: netdev; +Cc: bridge, roopa, stephen, Nikolay Aleksandrov
When vlan tunnels were introduced, vlan range errors got silently
dropped and instead 0 was returned always. Restore the previous
behaviour and return errors to user-space.
Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
err should be always set because we have a check for the vid range above
the loop and it must ensure we will go through the loop at least once
net/bridge/br_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 3bc890716c89..de2152730809 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -573,7 +573,7 @@ static int br_process_vlan_info(struct net_bridge *br,
}
*vinfo_last = NULL;
- return 0;
+ return err;
}
return br_vlan_info(br, p, cmd, vinfo_curr);
--
2.1.4
^ permalink raw reply related
* [PATCH 1/2] net: netrom: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 17:21 UTC (permalink / raw)
To: Ralf Baechle, David S. Miller
Cc: linux-hams, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/netrom/nr_route.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index 0c59354..fc9cadc 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -279,6 +279,7 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
nr_node->routes[1] = nr_node->routes[2];
nr_node->routes[2] = nr_route;
}
+ /* fall through */
case 2:
if (nr_node->routes[1].quality > nr_node->routes[0].quality) {
switch (nr_node->which) {
@@ -384,6 +385,7 @@ static int nr_del_node(ax25_address *callsign, ax25_address *neighbour, struct n
switch (i) {
case 0:
nr_node->routes[0] = nr_node->routes[1];
+ /* fall through */
case 1:
nr_node->routes[1] = nr_node->routes[2];
case 2:
@@ -506,7 +508,7 @@ static int nr_dec_obs(void)
switch (i) {
case 0:
s->routes[0] = s->routes[1];
- /* Fallthrough */
+ /* fall through */
case 1:
s->routes[1] = s->routes[2];
case 2:
@@ -553,6 +555,7 @@ void nr_rt_device_down(struct net_device *dev)
switch (i) {
case 0:
t->routes[0] = t->routes[1];
+ /* fall through */
case 1:
t->routes[1] = t->routes[2];
case 2:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] ipv6: let trace_fib6_table_lookup() dereference the fib table
From: Martin KaFai Lau @ 2017-10-19 17:26 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Steven Rostedt, Ingo Molnar, David Ahern
In-Reply-To: <48235df8fdf787356070d8baa1ac9816ef5736d4.1508398120.git.pabeni@redhat.com>
On Thu, Oct 19, 2017 at 07:31:43AM +0000, Paolo Abeni wrote:
> The perf traces for ipv6 routing code show a relevant cost around
> trace_fib6_table_lookup(), even if no trace is enabled. This is
> due to the fib6_table de-referencing currently performed by the
> caller.
>
> Let's the tracing code pay this overhead, passing to the trace
> helper the table pointer. This gives small but measurable
> performance improvement under UDP flood.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* [PATCH 2/2] net: netrom: refactor code in nr_add_node
From: Gustavo A. R. Silva @ 2017-10-19 17:27 UTC (permalink / raw)
To: Ralf Baechle, David S. Miller
Cc: linux-hams, netdev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <20171019172123.GA15219@embeddedor.com>
Code refactoring in order to make the code easier to read and maintain.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
net/netrom/nr_route.c | 63 ++++++++++++++++-----------------------------------
1 file changed, 20 insertions(+), 43 deletions(-)
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index fc9cadc..1e5165f 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -80,6 +80,23 @@ static struct nr_neigh *nr_neigh_get_dev(ax25_address *callsign,
static void nr_remove_neigh(struct nr_neigh *);
+/* re-sort the routes in quality order. */
+static inline void re_sort_routes(struct nr_node *nr_node, int ix_x, int ix_y)
+{
+ struct nr_route nr_route;
+
+ if (nr_node->routes[ix_y].quality > nr_node->routes[ix_x].quality) {
+ if (nr_node->which == ix_x)
+ nr_node->which = ix_y;
+ else if (nr_node->which == ix_y)
+ nr_node->which = ix_x;
+
+ nr_route = nr_node->routes[ix_x];
+ nr_node->routes[ix_x] = nr_node->routes[ix_y];
+ nr_node->routes[ix_y] = nr_route;
+ }
+}
+
/*
* Add a new route to a node, and in the process add the node and the
* neighbour if it is new.
@@ -90,7 +107,6 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
{
struct nr_node *nr_node;
struct nr_neigh *nr_neigh;
- struct nr_route nr_route;
int i, found;
struct net_device *odev;
@@ -251,50 +267,11 @@ static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
/* Now re-sort the routes in quality order */
switch (nr_node->count) {
case 3:
- if (nr_node->routes[1].quality > nr_node->routes[0].quality) {
- switch (nr_node->which) {
- case 0:
- nr_node->which = 1;
- break;
- case 1:
- nr_node->which = 0;
- break;
- }
- nr_route = nr_node->routes[0];
- nr_node->routes[0] = nr_node->routes[1];
- nr_node->routes[1] = nr_route;
- }
- if (nr_node->routes[2].quality > nr_node->routes[1].quality) {
- switch (nr_node->which) {
- case 1: nr_node->which = 2;
- break;
-
- case 2: nr_node->which = 1;
- break;
-
- default:
- break;
- }
- nr_route = nr_node->routes[1];
- nr_node->routes[1] = nr_node->routes[2];
- nr_node->routes[2] = nr_route;
- }
+ re_sort_routes(nr_node, 0, 1);
+ re_sort_routes(nr_node, 1, 2);
/* fall through */
case 2:
- if (nr_node->routes[1].quality > nr_node->routes[0].quality) {
- switch (nr_node->which) {
- case 0: nr_node->which = 1;
- break;
-
- case 1: nr_node->which = 0;
- break;
-
- default: break;
- }
- nr_route = nr_node->routes[0];
- nr_node->routes[0] = nr_node->routes[1];
- nr_node->routes[1] = nr_route;
- }
+ re_sort_routes(nr_node, 0, 1);
case 1:
break;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] wanxl: use m68k-linux-gnu-as if available
From: Krzysztof Halasa @ 2017-10-19 17:25 UTC (permalink / raw)
To: David Miller; +Cc: kilobyte, netdev
In-Reply-To: <20171018.130721.1208428223967129203.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> We don't even know if whatever "as68k" is would be the same thing
> as GNU as and generate the same binaries.
It's GNU as, likewise ld68k, though I have no idea if recent versions
would compile/link the firmware (correctly). This is 15+ years old.
I don't have opinion on the patch.
--
Krzysztof Halasa
^ permalink raw reply
* Re: [PATCH net-next 3/3] strparser: Generalize strparser
From: Eric Dumazet @ 2017-10-19 17:42 UTC (permalink / raw)
To: Tom Herbert; +Cc: netdev, rohit, john.fastabend
In-Reply-To: <20170728232243.3040-4-tom@quantonium.net>
On Fri, 2017-07-28 at 16:22 -0700, Tom Herbert wrote:
> Generalize strparser from more than just being used in conjunction
> with read_sock. strparser will also be used in the send path with
> zero proxy. The primary change is to create strp_process function
> that performs the critical processing on skbs. The documentation
> is also updated to reflect the new uses.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---
> Documentation/networking/strparser.txt | 207 +++++++++++++++-------
> include/net/strparser.h | 119 +++++++------
> net/kcm/kcmproc.c | 34 ++--
> net/kcm/kcmsock.c | 38 ++--
> net/strparser/strparser.c | 313 ++++++++++++++++++++-------------
> 5 files changed, 424 insertions(+), 287 deletions(-)
Just found this gem :
static void strp_msg_timeout(unsigned long arg)
{
struct strparser *strp = (struct strparser *)arg;
/* Message assembly timed out */
STRP_STATS_INCR(strp->stats.msg_timeouts);
strp->cb.lock(strp);
strp->cb.abort_parser(strp, ETIMEDOUT);
strp->cb.unlock(strp);
}
static void strp_sock_lock(struct strparser *strp)
{
lock_sock(strp->sk);
}
static void strp_sock_unlock(struct strparser *strp)
{
release_sock(strp->sk);
}
A timer runs from BH, and from this interrupt context it is absolutely
illegal to call lock_sock() ( and release_sock() )
Please fix, thanks !
^ permalink raw reply
* [PATCH] net: netrom: nr_in: mark expected switch fall-through
From: Gustavo A. R. Silva @ 2017-10-19 17:43 UTC (permalink / raw)
To: Ralf Baechle, David S. Miller
Cc: linux-hams, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/netrom/nr_in.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netrom/nr_in.c b/net/netrom/nr_in.c
index 80dbd0b..fbfdae4 100644
--- a/net/netrom/nr_in.c
+++ b/net/netrom/nr_in.c
@@ -125,7 +125,7 @@ static int nr_state2_machine(struct sock *sk, struct sk_buff *skb,
case NR_DISCREQ:
nr_write_internal(sk, NR_DISCACK);
-
+ /* fall through */
case NR_DISCACK:
nr_disconnect(sk, 0);
break;
--
2.7.4
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Paul Moore @ 2017-10-19 17:47 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Aleksa Sarai, James Bottomley, cgroups, mszeredi, Andy Lutomirski,
jlayton, Carlos O'Donell, API, Linux Containers, Linux Kernel,
Viro, David Howells, Linux FS Devel, linux-audit, Simo Sorce,
Development, Casey Schaufler, Eric Paris, Steve Grubb, trondmy
In-Reply-To: <87y3o7gl5l.fsf@xmission.com>
On Thu, Oct 19, 2017 at 12:25 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Paul Moore <paul@paul-moore.com> writes:
>
>> On Wed, Oct 18, 2017 at 8:43 PM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>> Aleksa Sarai <asarai@suse.de> writes:
>>>>>> The security implications are that anything that can change the label
>>>>>> could also hide itself and its doings from the audit system and thus
>>>>>> would be used as a means to evade detection. I actually think this
>>>>>> means the label should be write once (once you've set it, you can't
>>>>>> change it) ...
>>>>>
>>>>> Richard and I have talked about a write once approach, but the
>>>>> thinking was that you may want to allow a nested container
>>>>> orchestrator (Why? I don't know, but people always want to do the
>>>>> craziest things.) and a write-once policy makes that impossible. If
>>>>> we punt on the nested orchestrator, I believe we can seriously think
>>>>> about a write-once policy to simplify things.
>>>>
>>>> Nested containers are a very widely used use-case (see LXC system containers,
>>>> inside of which people run other container runtimes). So I would definitely
>>>> consider it something that "needs to be supported in some way". While the LXC
>>>> guys might be a *tad* crazy, the use-case isn't. :P
>>
>> No worries, we're all a little crazy in our own special ways ;)
>>
>> Kidding aside, thanks for explaining the use case.
>>
>>> Of course some of that gets to running auditd inside a container which
>>> we don't have yet either.
>>>
>>> So I think to start it is perfectly fine to figure out the non-nested
>>> case first and what makes sense there. Then to sort out the nested
>>> container case.
>>>
>>> The solution might be that a process gets at most one id per ``audit
>>> namespace''.
>>
>> In an attempt to stay on-topic, let's try to stick with "audit
>> container ID" or "container ID" if you must. I really want to avoid
>> the term "audit namespace" simply because the term "namespace" implies
>> some things which we aren't planning on doing.
>
> This is 100% on topic. I am saying that unless we are planing to have
> auditd running in a container with it's own set of rules you probably
> don't care about nested containers. Last time I heard a discussion
> about that the term in use was audit namespace. So I was referring to
> that support when I said audit namespace, even if the end result only
> loosely fits the term namespace.
My "stay on-topic" comment is directed at, and limited to, your choice
of terminology, not the discussion about container nesting. I'm
purposefully not using the term "audit namespace" to refer to anything
that Richard has presented, and I'm kindly asking you to do the same,
it simply doesn't fit.
> I could be wrong of course. I don't fully understand what is driving
> the desire to connect audit and containers. But my naive guess is that
> one from an audit perspective you don't care about nested containers
> unless there is also a nested auditd who is looking at it from a nested
> perspective.
Two motivations that are clear to me: the first is the desire to be
able to associate events in the audit log with a container (much like
how the session ID helped us associate events with a login session),
the second is the desire for users to run an audit daemon instance in
their containers to capture audit events generated by their container.
There is also a security certification motivation, see some of Steve's
comments for more on that.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH] openvswitch: conntrack: mark expected switch fall-through
From: Gustavo A. R. Silva @ 2017-10-19 17:55 UTC (permalink / raw)
To: Pravin Shelar, David S. Miller
Cc: netdev, dev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case I placed a "fall through" comment on
its own line, which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
net/openvswitch/conntrack.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index fe861e2..b27c5c6 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -752,6 +752,7 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct,
}
}
/* Non-ICMP, fall thru to initialize if needed. */
+ /* fall through */
case IP_CT_NEW:
/* Seen it before? This can happen for loopback, retrans,
* or local packets.
--
2.7.4
^ permalink raw reply related
* [PATCH] net: rose: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 18:03 UTC (permalink / raw)
To: Ralf Baechle, David S. Miller
Cc: linux-hams, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/rose/rose_in.c | 1 +
net/rose/rose_route.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0a63947..9bbbfe3 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -219,6 +219,7 @@ static int rose_state4_machine(struct sock *sk, struct sk_buff *skb, int framety
switch (frametype) {
case ROSE_RESET_REQUEST:
rose_write_internal(sk, ROSE_RESET_CONFIRMATION);
+ /* fall through */
case ROSE_RESET_CONFIRMATION:
rose_stop_timer(sk);
rose_start_idletimer(sk);
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 452bbb3..ac0f733 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -346,6 +346,7 @@ static int rose_del_node(struct rose_route_struct *rose_route,
case 0:
rose_node->neighbour[0] =
rose_node->neighbour[1];
+ /* fall through */
case 1:
rose_node->neighbour[1] =
rose_node->neighbour[2];
@@ -507,6 +508,7 @@ void rose_rt_device_down(struct net_device *dev)
switch (i) {
case 0:
t->neighbour[0] = t->neighbour[1];
+ /* fall through */
case 1:
t->neighbour[1] = t->neighbour[2];
case 2:
--
2.7.4
^ 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