* Re: [PATCH nf-next] netns: add and use net_ns_barrier
From: Eric W. Biederman @ 2017-05-31 17:46 UTC (permalink / raw)
To: David Miller; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20170531.125527.1553231867217469072.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Florian Westphal <fw@strlen.de>
> Date: Tue, 30 May 2017 11:38:12 +0200
>
>> Quoting Joe Stringer:
>> If a user loads nf_conntrack_ftp, sends FTP traffic through a network
>> namespace, destroys that namespace then unloads the FTP helper module,
>> then the kernel will crash.
>>
>> Events that lead to the crash:
>> 1. conntrack is created with ftp helper in netns x
>> 2. This netns is destroyed
>> 3. netns destruction is scheduled
>> 4. netns destruction wq starts, removes netns from global list
>> 5. ftp helper is unloaded, which resets all helpers of the conntracks
>> via for_each_net()
>>
>> but because netns is already gone from list the for_each_net() loop
>> doesn't include it, therefore all of these conntracks are unaffected.
>>
>> 6. helper module unload finishes
>> 7. netns wq invokes destructor for rmmod'ed helper
>>
>> CC: "Eric W. Biederman" <ebiederm@xmission.com>
>> Reported-by: Joe Stringer <joe@ovn.org>
>> Signed-off-by: Florian Westphal <fw@strlen.de>
>> ---
>> Eric, I'd like an explicit (n)ack from you for this one.
>
> Indeed, Eric, please do.
Taking a look now. The original didn't make it's way into my inbox. I
just have a copy from netdev. Florian there may be a bit of an email
black hole between us.
> Otherwise I'm fine with the generic parts:
>
> Acked-by: David S. Miller <davem@davemloft.net>
Eric
^ permalink raw reply
* Re: [PATCH] net: phy: fix marvell phy status reading
From: David Miller @ 2017-05-31 17:53 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <E1dFixz-00062r-0n@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Tue, 30 May 2017 16:21:51 +0100
> The Marvell driver incorrectly provides phydev->lp_advertising as the
> logical and of the link partner's advert and our advert. This is
> incorrect - this field is supposed to store the link parter's unmodified
> advertisment.
>
> This allows ethtool to report the correct link partner auto-negotiation
> status.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 1/1] netvsc: Add #include's for csum_* function declarations
From: David Miller @ 2017-05-31 17:59 UTC (permalink / raw)
To: mikelley
Cc: olaf, sthemmin, netdev, jasowang, linux-kernel, marcelo.cerri,
apw, devel, leann.ogasawara
In-Reply-To: <1496166184-19805-1-git-send-email-mikelley@microsoft.com>
From: Michael Kelley <mikelley@microsoft.com>
Date: Tue, 30 May 2017 10:43:04 -0700
> Add direct #include statements for declarations of csum_tcpudp_magic()
> and csum_ipv6_magic(). While the needed #include's are picked up
> indirectly for the x86 architecture, they aren't on other
> architectures, resulting in compile errors.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Applied, thank you.
^ permalink raw reply
* RE: [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc
From: Mintz, Yuval @ 2017-05-31 17:59 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1496240608-17453-1-git-send-email-baijiaju1990@163.com>
> The driver may sleep in interrupt handling, and the function call path is:
> qed_int_sp_dpc (tasklet_init indicates it handles interrupt)
> qed_int_attentions
> qed_mcp_handle_events
> qed_mcp_handle_link_change
> qed_link_update
> qed_fill_link
> qed_mcp_get_media_type
> qed_ptt_acquire
> usleep_range --> may sleep
>
> To fix it, the "usleep_range" is replaced with "udelay".
Thanks for pointing this out, but I believe the fix isn't the right one.
The better option would be to propagate the PTT from the outside
instead of acquiring a new one - in the call stack you've shared
qed_mcp_link_change() held a PTT entry and we could have propagated
it all the way through.
I'll try sending a fix for this in the next couple of days.
^ permalink raw reply
* [PATCH net-next 1/2] bpf: Allow CGROUP_SKB eBPF program to access sk_buff
From: Chenbo Feng @ 2017-05-31 18:06 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
This allows cgroup eBPF program to classify packet based on their
protocol or other detail information. Currently program need
CAP_NET_ADMIN privilege to attach a cgroup eBPF program, and A
process with CAP_NET_ADMIN can already see all packets on the system,
for example, by creating an iptables rules that causes the packet to
be passed to userspace via NFLOG.
Signed-off-by: Chenbo Feng <fengc@google.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 339c8a1..94a9bc9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2419,6 +2419,7 @@ static bool may_access_skb(enum bpf_prog_type type)
case BPF_PROG_TYPE_SOCKET_FILTER:
case BPF_PROG_TYPE_SCHED_CLS:
case BPF_PROG_TYPE_SCHED_ACT:
+ case BPF_PROG_TYPE_CGROUP_SKB:
return true;
default:
return false;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/2] bpf: Remove the capability check for cgroup skb eBPF program
From: Chenbo Feng @ 2017-05-31 18:06 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Chenbo Feng
In-Reply-To: <1496254013-18719-1-git-send-email-chenbofeng.kernel@gmail.com>
From: Chenbo Feng <fengc@google.com>
Currently loading a cgroup skb eBPF program require a CAP_SYS_ADMIN
capability while attaching the program to a cgroup only requires the
user have CAP_NET_ADMIN privilege. We can escape the capability
check when load the program just like socket filter program to make
the capability requirement consistent.
Signed-off-by: Chenbo Feng <fengc@google.com>
---
kernel/bpf/syscall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 265a0d8..021c0b4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -815,7 +815,9 @@ static int bpf_prog_load(union bpf_attr *attr)
attr->kern_version != LINUX_VERSION_CODE)
return -EINVAL;
- if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
+ if (type != BPF_PROG_TYPE_SOCKET_FILTER
+ && type != BPF_PROG_TYPE_CGROUP_SKB
+ && !capable(CAP_SYS_ADMIN))
return -EPERM;
/* plain bpf_prog allocation */
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
From: Andy Shevchenko @ 2017-05-31 18:09 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <1ec11df8a4860215c93125f83a04c8c00482a041.1496129604.git.jan.kiszka@siemens.com>
On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.
> struct stmmac_pci_dmi_data {
> - const char *name;
> - const char *asset_tag;
> - unsigned int func;
> + int func;
> int phy_addr;
> };
Can we use the following instead:
struct stmmac_pci_dmi_data {
unsigned int *func;
int *phy_addr;
size_t nfuncs;
};
or something like
struct stmmac_pci_func_data {
unsigned int func;
int phy_addr;
};
struct stmmac_pci_dmi_data {
struct stmmac_pci_func_data *func;
size_t nfuncs;
};
(Latter would be better since it allows to use ARRAY_SIZE() and less
error prone for possible asymmetrical amount of values in the former)
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH nf-next] netns: add and use net_ns_barrier
From: Eric W. Biederman @ 2017-05-31 18:13 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev
In-Reply-To: <20170530093812.10712-1-fw@strlen.de>
Florian Westphal <fw@strlen.de> writes:
> Quoting Joe Stringer:
> If a user loads nf_conntrack_ftp, sends FTP traffic through a network
> namespace, destroys that namespace then unloads the FTP helper module,
> then the kernel will crash.
>
> Events that lead to the crash:
> 1. conntrack is created with ftp helper in netns x
> 2. This netns is destroyed
> 3. netns destruction is scheduled
> 4. netns destruction wq starts, removes netns from global list
> 5. ftp helper is unloaded, which resets all helpers of the conntracks
> via for_each_net()
>
> but because netns is already gone from list the for_each_net() loop
> doesn't include it, therefore all of these conntracks are unaffected.
>
> 6. helper module unload finishes
> 7. netns wq invokes destructor for rmmod'ed helper
>
> CC: "Eric W. Biederman" <ebiederm@xmission.com>
> Reported-by: Joe Stringer <joe@ovn.org>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Eric, I'd like an explicit (n)ack from you for this one.
This doesn't look too scary but I have the impression we have addressed
this elsewhere with a different solution.
Looking...
Ok. unregister_pernet_operations takes the net_mutex and thus
gives you this barrier automatically.
Hmm. Why isn't this working for conntrack, looking...
nf_conntrack_ftp doesn't use unregister_pernet_operations...
nf_conntract_ftp does use nf_conntrack_helpers_unregister
I think I almost see the problem.
What is the per net code that stops dealing with the nf_conntract_ftp?
I am trying to figure out if your netns_barrier is reasonable or if
it treating the symptom. I am having trouble seeing enough of what
conntrack is doing to judge.
Am I correct in understanding that the root problem is there is
something pointing to ftp_exp_policy at the time of module unload?
Eric
> include/net/net_namespace.h | 3 +++
> net/core/net_namespace.c | 17 +++++++++++++++++
> net/netfilter/nf_conntrack_core.c | 9 +++++++++
> 3 files changed, 29 insertions(+)
>
> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
> index fe80bb48ab1f..a24a57593202 100644
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -158,6 +158,7 @@ extern struct net init_net;
> struct net *copy_net_ns(unsigned long flags, struct user_namespace *user_ns,
> struct net *old_net);
>
> +void net_ns_barrier(void);
> #else /* CONFIG_NET_NS */
> #include <linux/sched.h>
> #include <linux/nsproxy.h>
> @@ -168,6 +169,8 @@ static inline struct net *copy_net_ns(unsigned long flags,
> return ERR_PTR(-EINVAL);
> return old_net;
> }
> +
> +static inline void net_ns_barrier(void) {}
> #endif /* CONFIG_NET_NS */
>
>
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 1934efd4a9d4..1f15abb1d733 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -482,6 +482,23 @@ static void cleanup_net(struct work_struct *work)
> net_drop_ns(net);
> }
> }
> +
> +/**
> + * net_ns_barrier - wait until concurrent net_cleanup_work is done
> + *
> + * cleanup_net runs from work queue and will first remove namespaces
> + * from the global list, then run net exit functions.
> + *
> + * Call this in module exit path to make sure that all netns
> + * ->exit ops have been invoked before the function is removed.
> + */
> +void net_ns_barrier(void)
> +{
> + mutex_lock(&net_mutex);
> + mutex_unlock(&net_mutex);
> +}
> +EXPORT_SYMBOL(net_ns_barrier);
> +
> static DECLARE_WORK(net_cleanup_work, cleanup_net);
>
> void __put_net(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index c3bd9b086dcc..ee972ee7bf81 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1720,6 +1720,8 @@ EXPORT_SYMBOL_GPL(nf_ct_iterate_cleanup_net);
> * Like nf_ct_iterate_cleanup, but first marks conntracks on the
> * unconfirmed list as dying (so they will not be inserted into
> * main table).
> + *
> + * Can only be called in module exit path.
> */
> void
> nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
> @@ -1734,6 +1736,13 @@ nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
> }
> rtnl_unlock();
>
> + /* Need to wait for netns cleanup worker to finish, if its
> + * running -- it might have deleted a net namespace from
> + * the global list, so our __nf_ct_unconfirmed_destroy() might
> + * not have affected all namespaces.
> + */
> + net_ns_barrier();
> +
> /* a conntrack could have been unlinked from unconfirmed list
> * before we grabbed pcpu lock in __nf_ct_unconfirmed_destroy().
> * This makes sure its inserted into conntrack table.
^ permalink raw reply
* Re: [PATCH net-next 6/9] bpf: fix stack_depth usage by test_bpf.ko
From: David Miller @ 2017-05-31 18:15 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, kernel-team
In-Reply-To: <20170530203135.3642768-7-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Tue, 30 May 2017 13:31:32 -0700
> test_bpf.ko doesn't call verifier before selecting interpreter or JITing,
> hence the tests need to manually specify the amount of stack they consume.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
I do not like this and the previous patch, it seems so error prone.
^ permalink raw reply
* Re: loosing netdevices with namespaces and unshare?
From: Harald Welte @ 2017-05-31 18:11 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpU517NR1xsrDrAohnESdS33WoToHsdGD0yOWSiGU9Yiqg@mail.gmail.com>
Hi Cong,
On Wed, May 31, 2017 at 10:44:53AM -0700, Cong Wang wrote:
> >> Net namespace simply unregisters all netdevices inside when it is
> >> gone, no matter where they are from.
> >
> > ah, ok. I missed that part. Is there a good piece of documentation on
> > netwokr namespaces that I should read?
>
> I don't know any doc mentioning this.
That's of course a pity. I'll see what can be done about amending the
netns related manpage or the like.
> >> > Of course I know I could simply do something like "ip link set eth0
> >> > netns 1" from within the namespace before leaving. But what if the
> >> > process is not bash and the process exits abnormally? I'd consider
> >> > that explicit reassignment more like a hack than a proper solution...
> >>
> >> It doesn't make sense to move it back to where it is from, for example,
> >> what if you move a veth0 from netns1 to netns2 and netns1 is gone
> >> before netns2?
> >
> > for virtual devices, I would agree. For physical devices, I think the
> > default behavior to unregister them is - from my of course very
> > subjective point of view - quite questionable.
>
> Network namespace does not special-case the physical devices,
> it treats them all equally as abstract net devices.
I hear you, and I understand that of course from a developer point of
view it makes sense to treat all devices the same. I just wonder if
from an usability point of view this is the best choice. Virtual
devices can be (re)created at any time, physical not.
I mean, what is the *use case* for loosing any refrence to a physical
network device and unregistering it from the stack? Is there any API by
which a new netdevice structure can be instantiated on the actual
hardware? Registering the netdev is what the driver does during
discovering the system hardware. If there's a method to "automagically"
loose devices, at the very least I wold expect some reasonable method to
resurrect them. Unloading the kernel module and reloading it is for
sure not elegant, particularly not if you have multiple Ethernet
devices/ports sharing the same driver.
One could e.g. also think of something like a special namespace that
collects all the "orphan" netdevices. Something analogous to the old
Unix tradition of "pid 1" collecting all the orphan tasks whose parents
died. Transferring them into that "netdev orphanage" could
automatically set the link down so that no accidential
routing/forwarding of traffic between the devices is possible.
This is just my two cents. Given my past involvement in Linux
networking I allow myself having an opinion on such matters. But if the
kernel networking community thinks it is ok to loose all references to a
physical network device due to processes terminating irregularly (which
will happen, as indicated in OOM or software bug cases), then I will of
course have to accept that.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] net: dsa: factor skb freeing on xmit
From: Vivien Didelot @ 2017-05-31 18:18 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <8487d628-874e-cc45-a15a-07725930dbc2@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 05/30/2017 11:33 AM, Vivien Didelot wrote:
>> The taggers are currently responsible to free the original SKB if they
>> made a copy of it, or in case of error.
>>
>> This patch simplifies this by freeing the original SKB in the
>> dsa_slave_xmit caller, but only if an error (NULL) is returned.
>
> Is not it a clearer contract if the tagging protocol must always free
> the original SKB, whether it succeeded in creating a new one or not?
The rcv caller frees the original SKB on error, as of a86d8becc3f0
("net: dsa: Factor bottom tag receive functions").
It'd be nice if we could make both xmit and rcv symmetric, and limit the
tagging implementations to only do their real job: tag/untag an SKB with
the destination/source port.
Thanks,
Vivien
^ permalink raw reply
* Re: [patch net-next 0/9] mlxsw: Support firmware flash
From: David Miller @ 2017-05-31 18:18 UTC (permalink / raw)
To: gerlitz.or; +Cc: yotamg, Yuval.Mintz, jiri, netdev, idosch, mlxsw, bhutchings
In-Reply-To: <CAJ3xEMi_U-139_EXfuT+d0PLyBOn6C+aWaGpFfmH6E8odhtvaw@mail.gmail.com>
From: Or Gerlitz <gerlitz.or@gmail.com>
Date: Tue, 30 May 2017 23:32:22 +0300
> On Sun, May 28, 2017 at 10:26 AM, Yotam Gigi <yotamg@mellanox.com> wrote:
>> On 05/23/2017 06:38 PM, David Miller wrote:
>>> From: Yotam Gigi <yotamg@mellanox.com>
>>> Date: Tue, 23 May 2017 18:14:15 +0300
>
>>>> Sorry, I am not sure I understand. You think that drivers should not implement
>>>> ethtool's flash_device callback anymore? do you have an alternative for
>>>> firmware flash?
>
>>> As stated, export an MTD device.
>
>> So, after we have been going over MTD, it seems like it does not fit our needs
>> at all.
>
>> MTD device provides (erasable-)block access to a flash storage, where in our
>> case the firmware burn process is just pouring a binary BLOB into the device.
>> The driver is not aware of the internal storage used for storing the firmware as
>> it is not defined in our driver-hardware API.
>>
>> Needless to say that block access has no meaning in our case, so any solution
>> that will involve MTD device to burn our firmware (if there is a solution at
>> all) will be a workaround and will not fit MTD purpose.
>>
>> Apart for boot time firmware flash, which we have already pushed we would really
>> like to allow the user to ask for a specific firmware version. Do you have any
>> other solution for us apart from "ethtool -f"?
>>
>> This problem is even more relevant in the Mellanox HCA driver team, which would
>> like to use that code in order to burn the HCA firmware, but not intend to
>> trigger it on boot time, which means that must have a way for the user to
>> trigger it.
>
>
> Hi Dave,
>
> We had few more emails on this thread with Jakub, and he's now happy
> with our replies, so where do we go from here? could you comment on
> Yotam's note.
Ok, use ethtool -f if you must.
^ permalink raw reply
* Re: [PATCH] net: fix auto-loading of Marvell DSA driver
From: David Miller @ 2017-05-31 18:19 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <E1dFnuE-0006U0-PM@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Tue, 30 May 2017 21:38:18 +0100
> Auto-loading of the Marvell DSA driver has stopped working with recent
> kernels. This seems to be due to the change of binding for DSA devices,
> moving them from the platform bus to the MDIO bus.
>
> In order for module auto-loading to work, we need to provide a MODALIAS
> string in the uevent file for the device. However, the device core does
> not automatically provide this, and needs each bus_type to implement a
> uevent method to generate these strings. The MDIO bus does not provide
> such a method, so no MODALIAS string is provided:
>
> .# cat /sys/bus/mdio_bus/devices/f1072004.mdio-mii\:04/uevent
> DRIVER=mv88e6085
> OF_NAME=switch
> OF_FULLNAME=/soc/internal-regs/mdio@72004/switch@4
> OF_COMPATIBLE_0=marvell,mv88e6085
> OF_COMPATIBLE_N=1
>
> In the case of OF-based devices, the solution is easy -
> of_device_uevent_modalias() does the work for us. After this is done,
> the uevent file looks like this:
>
> .# cat /sys/bus/mdio_bus/devices/f1072004.mdio-mii\:04/uevent
> DRIVER=mv88e6085
> OF_NAME=switch
> OF_FULLNAME=/soc/internal-regs/mdio@72004/switch@4
> OF_COMPATIBLE_0=marvell,mv88e6085
> OF_COMPATIBLE_N=1
> MODALIAS=of:NswitchT<NULL>Cmarvell,mv88e6085
>
> which results in auto-loading of the Marvell DSA driver on Clearfog
> platforms.
>
> Fixes: c0405563a613 ("ARM: dts: armada-388-clearfog: Utilize new DSA binding")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH V2 net-next] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: David Miller @ 2017-05-31 18:21 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: netdev, stephen
In-Reply-To: <1496179897-82646-1-git-send-email-sowmini.varadhan@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Tue, 30 May 2017 14:31:37 -0700
> +EXPORT_SYMBOL(neigh_remove_one);
net/ipv4/arp.c is the only reference you add to this symbol and ipv4
cannot be built modular.
So this export is unnecessary.
^ permalink raw reply
* [PATCH net] tcp: disallow cwnd undo when switching congestion control
From: Yuchung Cheng @ 2017-05-31 18:21 UTC (permalink / raw)
To: davem; +Cc: netdev, ncardwell, edumazet, soheil, Yuchung Cheng
When the sender switches its congestion control during loss
recovery, if the recovery is spurious then it may incorrectly
revert cwnd and ssthresh to the older values set by a previous
congestion control. Consider a congestion control (like BBR)
that does not use ssthresh and keeps it infinite: the connection
may incorrectly revert cwnd to an infinite value when switching
from BBR to another congestion control.
This patch fixes it by disallowing such cwnd undo operation
upon switching congestion control. Note that undo_marker
is not reset s.t. the packets that were incorrectly marked
lost would be corrected. We only avoid undoing the cwnd in
tcp_undo_cwnd_reduction().
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_cong.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 6e3c512054a6..324c9bcc5456 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -180,6 +180,7 @@ void tcp_init_congestion_control(struct sock *sk)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
+ tcp_sk(sk)->prior_ssthresh = 0;
if (icsk->icsk_ca_ops->init)
icsk->icsk_ca_ops->init(sk);
if (tcp_ca_needs_ecn(sk))
--
2.13.0.219.gdb65acc882-goog
^ permalink raw reply related
* Re: [PATCH] net: freescale: fix potential null pointer dereference
From: David Miller @ 2017-05-31 18:24 UTC (permalink / raw)
To: garsilva; +Cc: netdev, linux-kernel
In-Reply-To: <20170530223842.GA11915@embeddedgus>
From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Tue, 30 May 2017 17:38:43 -0500
> Add NULL check before dereferencing pointer _id_ in order to avoid
> a potential NULL pointer dereference.
>
> Addresses-Coverity-ID: 1397995
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v5 net-next 0/5] dsa: add Microchip KSZ9477 DSA driver
From: David Miller @ 2017-05-31 18:28 UTC (permalink / raw)
To: Woojung.Huh; +Cc: andrew, f.fainelli, netdev, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40AA4D4C@CHN-SV-EXMX02.mchp-main.com>
From: <Woojung.Huh@microchip.com>
Date: Wed, 31 May 2017 00:00:34 +0000
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> This series of patches is for Microchip KSZ9477 DSA driver.
> KSZ9477 is 7 ports GigE switch with numerous advanced features.
> 5 ports are 10/100/1000 Mbps internal PHYs and 2 ports have
> Interfaces to SGMII, RGMII, MII or RMII.
>
> This patch supports VLAN, MDB, FDB and port mirroring offloads.
>
> Welcome reviews and comments from community.
>
> Note: Tests are performed on internal development board.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/8] Introduce bpf ID
From: David Miller @ 2017-05-31 18:30 UTC (permalink / raw)
To: kafai; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <cover.1496189155.git.kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Tue, 30 May 2017 17:08:29 -0700
> This patch series:
> 1) Introduce ID for both bpf_prog and bpf_map.
> 2) Add bpf commands to iterate the prog IDs and map
> IDs of the system.
> 3) Add bpf commands to get a prog/map fd from an ID
> 4) Add bpf command to get prog/map info from a fd.
> The prog/map info is a jump start in this patchset
> and it is not meant to be a complete list. They can
> be extended in the future patches.
This needs a respin to address the kbuild bot build failures.
^ permalink raw reply
* [PATCH net-next] tcp: use TS opt on RTTs for congestion control
From: Yuchung Cheng @ 2017-05-31 18:30 UTC (permalink / raw)
To: davem; +Cc: netdev, ncardwell, edumazet, soheil, Yuchung Cheng
Currently when a data packet is retransmitted, we do not compute an
RTT sample for congestion control due to Kern's check. Therefore the
congestion control that uses RTT signals may not receive any update
during loss recovery which could last many round trips. For example,
BBR and Vegas may not be able to update its min RTT estimation if the
network path has shortened until it recovers from losses. This patch
mitigates that by using TCP timestamp options for RTT measurement
for congestion control. Note that we already use timestamps for
RTT estimation.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_input.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9f4380662196..4ea8ec5c7bb4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2921,9 +2921,9 @@ static void tcp_update_rtt_min(struct sock *sk, u32 rtt_us)
rtt_us ? : jiffies_to_usecs(1));
}
-static inline bool tcp_ack_update_rtt(struct sock *sk, const int flag,
- long seq_rtt_us, long sack_rtt_us,
- long ca_rtt_us)
+static bool tcp_ack_update_rtt(struct sock *sk, const int flag,
+ long seq_rtt_us, long sack_rtt_us,
+ long ca_rtt_us, struct rate_sample *rs)
{
const struct tcp_sock *tp = tcp_sk(sk);
@@ -2948,6 +2948,7 @@ static inline bool tcp_ack_update_rtt(struct sock *sk, const int flag,
seq_rtt_us = ca_rtt_us = delta_us;
}
+ rs->rtt_us = ca_rtt_us; /* RTT of last (S)ACKed packet (or -1) */
if (seq_rtt_us < 0)
return false;
@@ -2967,12 +2968,13 @@ static inline bool tcp_ack_update_rtt(struct sock *sk, const int flag,
/* Compute time elapsed between (last) SYNACK and the ACK completing 3WHS. */
void tcp_synack_rtt_meas(struct sock *sk, struct request_sock *req)
{
+ struct rate_sample rs;
long rtt_us = -1L;
if (req && !req->num_retrans && tcp_rsk(req)->snt_synack)
rtt_us = tcp_stamp_us_delta(tcp_clock_us(), tcp_rsk(req)->snt_synack);
- tcp_ack_update_rtt(sk, FLAG_SYN_ACKED, rtt_us, -1L, rtt_us);
+ tcp_ack_update_rtt(sk, FLAG_SYN_ACKED, rtt_us, -1L, rtt_us, &rs);
}
@@ -3177,9 +3179,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
sack_rtt_us = tcp_stamp_us_delta(tp->tcp_mstamp, sack->first_sackt);
ca_rtt_us = tcp_stamp_us_delta(tp->tcp_mstamp, sack->last_sackt);
}
- sack->rate->rtt_us = ca_rtt_us; /* RTT of last (S)ACKed packet, or -1 */
rtt_update = tcp_ack_update_rtt(sk, flag, seq_rtt_us, sack_rtt_us,
- ca_rtt_us);
+ ca_rtt_us, sack->rate);
if (flag & FLAG_ACKED) {
tcp_rearm_rto(sk);
@@ -3215,7 +3216,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
if (icsk->icsk_ca_ops->pkts_acked) {
struct ack_sample sample = { .pkts_acked = pkts_acked,
- .rtt_us = ca_rtt_us,
+ .rtt_us = sack->rate->rtt_us,
.in_flight = last_in_flight };
icsk->icsk_ca_ops->pkts_acked(sk, &sample);
--
2.13.0.219.gdb65acc882-goog
^ permalink raw reply related
* hi
From: griestkrist @ 2017-05-31 18:32 UTC (permalink / raw)
--
Please contact me in the my e-mail addres,(griestkrist1983@gmail.com)
^ permalink raw reply
* [PATCH net-next] mlxsw: spectrum: Add bridge dependency for spectrum
From: Ido Schimmel @ 2017-05-31 18:33 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, nikolay, mlxsw, Ido Schimmel, Arnd Bergmann
When BRIDGE is a loadable module, MLXSW_SPECTRUM mustn't be built-in:
drivers/built-in.o: In function `mlxsw_sp_bridge_device_create':
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:145: undefined reference to `br_vlan_enabled'
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:158: undefined reference to `br_multicast_enabled'
drivers/built-in.o: In function `mlxsw_sp_dev_rif_type':
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:2972: undefined reference to `br_vlan_enabled'
drivers/built-in.o: In function `mlxsw_sp_inetaddr_vlan_event':
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:3310: undefined reference to `br_vlan_enabled'
Add Kconfig dependency to enforce usable configurations.
Fixes: c57529e1d5d8 ("mlxsw: spectrum: Replace vPorts with Port-VLAN")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Tested-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9f80c2..695adff 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -74,6 +74,7 @@ config MLXSW_SPECTRUM
tristate "Mellanox Technologies Spectrum support"
depends on MLXSW_CORE && MLXSW_PCI && NET_SWITCHDEV && VLAN_8021Q
depends on PSAMPLE || PSAMPLE=n
+ depends on BRIDGE || BRIDGE=n
select PARMAN
select MLXFW
default m
--
2.9.4
^ permalink raw reply related
* Re: [PATCH net-next] netlink: include netnsid only when netns differs.
From: Flavio Leitner @ 2017-05-31 18:34 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev
In-Reply-To: <c4250fae-3bf8-f646-9a68-de91419b0b8b@6wind.com>
On Wed, May 31, 2017 at 03:48:06PM +0200, Nicolas Dichtel wrote:
> Le 31/05/2017 à 14:28, Flavio Leitner a écrit :
> > On Wed, May 31, 2017 at 10:38:21AM +0200, Nicolas Dichtel wrote:
> >> Le 30/05/2017 à 23:33, Flavio Leitner a écrit :
> >>> Don't include netns id for notifications broadcasts when the
> >>> socket and the skb are in the same netns because it will be
> >>> an error which can't be distinguished from a peer netns failing
> >>> to allocate an id.
> >> I don't understand the problem. peernet2id() doesn't allocate ids, it only do a
> >> lookup. If you need an id for the current netns, you have to allocate one.
> >
> > The issue is that if you query an interface on the same netns, the
> > error is returned, then we cannot tell if the iface is on the same
> > netns or if there was an error while allocating the ID and the
> > iface is on another netns.
> If the returned id is NETNSA_NSID_NOT_ASSIGNED, then the netns is the same.
>
> Some lines before your patch, we call peernet_has_id() when the netns differ,
> thus we ensure that the id is available.
Right, but that's internal to the kernel.
> The principle was that netlink messages of other netns can be sent only if an id
> is assigned.
OK, could you please update include/uapi/linux/net_namespace.h to reflect that?
It says NETNSA_NSID_NOT_ASSIGNED are attributes for RTM_NEWNSID or RTM_GETNSID
which makes sense, but NOT_ASSIGNED sounds little like SAME_NSID for other
message types.
--
Flavio
^ permalink raw reply
* Re: [PATCH] enic: Fix a sleep-in-atomic bug
From: David Miller @ 2017-05-31 18:37 UTC (permalink / raw)
To: baijiaju1990; +Cc: benve, _govind, neepatel, netdev, linux-kernel
In-Reply-To: <1496198089-1596-1-git-send-email-baijiaju1990@163.com>
From: Jia-Ju Bai <baijiaju1990@163.com>
Date: Wed, 31 May 2017 10:34:49 +0800
> The driver may sleep under a spin lock in some function call paths.
> The 1st function call path is:
> enic_reset (acquire the lock by spin_lock)
> enic_stop
> enic_synchronize_irqs
> synchronize_irq --> may sleep
> The 2nd function call path is:
> enic_reset (acquire the lock by spin_lock)
> enic_dev_soft_reset
> enic_dev_wait
> schedule_timeout_uninterruptible --> may sleep
> The 3rd function call path is:
> enic_reset (acquire the lock by spin_lock)
> enic_open
> enic_request_intr
> enic_set_rx_cpu_rmap
> enic_free_rx_cpu_rmap
> free_irq_cpu_rmap --> may sleep
>
> To fix it, the "spin_lock" and "spin_unlock" are removed in enic_reset.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
That's not necessarily safe at all, the lock is probably needed for
synchronization.
^ permalink raw reply
* Re: [PATCH] enic: Fix another sleep-in-atomic bug
From: David Miller @ 2017-05-31 18:37 UTC (permalink / raw)
To: baijiaju1990; +Cc: benve, _govind, neepatel, netdev, linux-kernel
In-Reply-To: <1496198620-1840-1-git-send-email-baijiaju1990@163.com>
From: Jia-Ju Bai <baijiaju1990@163.com>
Date: Wed, 31 May 2017 10:43:40 +0800
> The driver may sleep under a spin lock in some function call paths.
> The 1st function call path is:
> enic_tx_hang_reset (acquire the lock by spin_lock)
> enic_stop
> enic_synchronize_irqs
> synchronize_irq --> may sleep
> The 2nd function call path is:
> enic_tx_hang_reset (acquire the lock by spin_lock)
> enic_dev_wait
> schedule_timeout_uninterruptible --> may sleep
> The 3rd function call path is:
> enic_tx_hang_reset (acquire the lock by spin_lock)
> enic_open
> enic_request_intr
> enic_set_rx_cpu_rmap
> enic_free_rx_cpu_rmap
> free_irq_cpu_rmap --> may sleep
>
> To fix it, the "spin_lock" and "spin_unlock" are removed
> in enic_tx_hang_reset.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
Likewise, you can't just blindly remove locks to fix a warning like
this.
^ permalink raw reply
* Re: [PATCH] isdn: Fix a sleep-in-atomic bug
From: David Miller @ 2017-05-31 18:38 UTC (permalink / raw)
To: baijiaju1990; +Cc: isdn, geliangtang, keescook, netdev, linux-kernel
In-Reply-To: <1496194811-32297-1-git-send-email-baijiaju1990@163.com>
From: Jia-Ju Bai <baijiaju1990@163.com>
Date: Wed, 31 May 2017 09:40:11 +0800
> The driver may sleep under a spin lock, the function call path is:
> isdn_ppp_mp_receive (acquire the lock)
> isdn_ppp_mp_reassembly
> isdn_ppp_push_higher
> isdn_ppp_decompress
> isdn_ppp_ccp_reset_trans
> isdn_ppp_ccp_reset_alloc_state
> kzalloc(GFP_KERNEL) --> may sleep
>
> To fixed it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
Applied.
^ 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