* Re: [PATCH net-next 1/2] net: tap: replay while() loop with % operator in tap_get_queue()
From: Stephen Hemminger @ 2017-12-11 16:58 UTC (permalink / raw)
To: yuan linyu; +Cc: netdev, David S . Miller, yuan linyu
In-Reply-To: <1512998801-17852-1-git-send-email-cugyly@163.com>
On Mon, 11 Dec 2017 21:26:41 +0800
yuan linyu <cugyly@163.com> wrote:
> From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
>
> Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> ---
> drivers/net/tap.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 0a886fda..78900a0 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -275,11 +275,7 @@ static struct tap_queue *tap_get_queue(struct tap_dev *tap,
>
> if (likely(skb_rx_queue_recorded(skb))) {
> rxq = skb_get_rx_queue(skb);
> -
> - while (unlikely(rxq >= numvtaps))
> - rxq -= numvtaps;
> -
> - queue = rcu_dereference(tap->taps[rxq]);
> + queue = rcu_dereference(tap->taps[rxq % numvtaps]);
> goto out;
> }
>
Modulus is slower than the loop.
^ permalink raw reply
* Re: [PATCH V2] netlink: Add netns check on taps
From: David Miller @ 2017-12-11 16:58 UTC (permalink / raw)
To: mkubecek; +Cc: cernekee, johannes.berg, netdev, linux-kernel, daniel, dsahern
In-Reply-To: <20171211161350.s5ki6peee2oxpric@unicorn.suse.cz>
From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 11 Dec 2017 17:13:50 +0100
> On Wed, Dec 06, 2017 at 03:57:14PM -0500, David Miller wrote:
>> From: Kevin Cernekee <cernekee@chromium.org>
>> Date: Wed, 6 Dec 2017 12:12:27 -0800
>>
>> > Currently, a nlmon link inside a child namespace can observe systemwide
>> > netlink activity. Filter the traffic so that nlmon can only sniff
>> > netlink messages from its own netns.
>> >
>> > Test case:
>> >
>> > vpnns -- bash -c "ip link add nlmon0 type nlmon; \
>> > ip link set nlmon0 up; \
>> > tcpdump -i nlmon0 -q -w /tmp/nlmon.pcap -U" &
>> > sudo ip xfrm state add src 10.1.1.1 dst 10.1.1.2 proto esp \
>> > spi 0x1 mode transport \
>> > auth sha1 0x6162633132330000000000000000000000000000 \
>> > enc aes 0x00000000000000000000000000000000
>> > grep --binary abc123 /tmp/nlmon.pcap
>> >
>> > Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>
>> Applied and queued up for -stable, thanks Kevin.
>
> David,
>
> this patch is marked as accepted in patchworks and listed in
>
> http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
>
> but it's not in the net tree. Is there a problem with it?
Sorry, it should be there now.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
From: David Miller @ 2017-12-11 17:01 UTC (permalink / raw)
To: jiri; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211163246.GC1885@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 11 Dec 2017 17:32:46 +0100
> I think that it does not make sense to convert ethtool->netlink_ethtool
> 1:1 feature wise. Now we have devlink, ritch switch representation
> model, tc offload and many others. Lot of things that are in
> ethtool, should be done in devlink. Also, there are couple of things
> that should just die - nice example is ethtool --config-ntuple - we
> should use tc for that.
Whilst I do agree that devlink is probably a good place for this stuff
(we want to be able to do ethetool things on objects that lack a netdev)
I do not agree with the tc angle.
It is entirely appropriate to set the ntuple settings of a driver
without being required to use TC or similar.
All you are going to do with your suggestion is make people keep using
the existing ethtool ioctl, because they'll say "screw this, I'm not
using TC I have something which works just fine already". And that's
not the goal of putting this stuff into netlink, we want people to
use the new facilities and move off of the ioctl.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: tap: replay while() loop with % operator in tap_get_queue()
From: David Miller @ 2017-12-11 17:02 UTC (permalink / raw)
To: stephen; +Cc: cugyly, netdev, Linyu.Yuan
In-Reply-To: <20171211085811.00c3cbaf@xeon-e3>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 11 Dec 2017 08:58:11 -0800
> On Mon, 11 Dec 2017 21:26:41 +0800
> yuan linyu <cugyly@163.com> wrote:
>
>> From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
>>
>> Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
>> ---
>> drivers/net/tap.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 0a886fda..78900a0 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -275,11 +275,7 @@ static struct tap_queue *tap_get_queue(struct tap_dev *tap,
>>
>> if (likely(skb_rx_queue_recorded(skb))) {
>> rxq = skb_get_rx_queue(skb);
>> -
>> - while (unlikely(rxq >= numvtaps))
>> - rxq -= numvtaps;
>> -
>> - queue = rcu_dereference(tap->taps[rxq]);
>> + queue = rcu_dereference(tap->taps[rxq % numvtaps]);
>> goto out;
>> }
>>
>
> Modulus is slower than the loop.
Agreed, the loop is there intentionally. We do this in other areas of
the kernel as well.
^ permalink raw reply
* Re: 76f43b4 fix for stable
From: David Miller @ 2017-12-11 17:03 UTC (permalink / raw)
To: rschuetz-82PuzOe0+4+KVelC9LqlCA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c0a512a7-e615-bbab-5c88-4a85946fefb5-82PuzOe0+4+KVelC9LqlCA@public.gmane.org>
From: Richard Schütz <rschuetz-82PuzOe0+4+KVelC9LqlCA@public.gmane.org>
Date: Mon, 11 Dec 2017 14:51:33 +0100
> as per netdev-FAQ.txt I'm requesting the submission of commit
> 57629915d568c522ac1422df7bba4bee5b5c7a7c ("mac80211: Fix addition of
> mesh configuration element") to stable. Because of automatic selection
> commit 76f43b4c0a9337af22827d78de4f2b8fd5328489 ("mac80211: Remove
> invalid flag operations in mesh TSF synchronization") was introduced
> into stable recently without this accompanying fix.
Johannes, please take care of this.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/4] nfp: dead code, clean ups and slight improvements
From: David Miller @ 2017-12-11 17:12 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20171209033704.5553-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 8 Dec 2017 19:37:00 -0800
> This series contains small clean ups from John and Carl, and brings
> no functional changes.
>
> John's improvements target the flower code. First he makes sure we don't
> allocate space in FW request messages for MAC matches if the TC rule does
> not contain any. The remaining two patches remove some dead code and
> unused defines.
>
> Carl follows up with a slight optimization to his recent ethtool FW state
> dumps, byte swapping input parameters once instead of the data for every
> dumped item.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v2] net: ethernet: arc: fix error handling in emac_rockchip_probe
From: David Miller @ 2017-12-11 17:13 UTC (permalink / raw)
To: branislav; +Cc: heiko, netdev, linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <20171209115133.19205-1-branislav@radocaj.org>
From: Branislav Radocaj <branislav@radocaj.org>
Date: Sat, 9 Dec 2017 12:51:33 +0100
> If clk_set_rate() fails, we should disable clk before return.
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Branislav Radocaj <branislav@radocaj.org>
> ---
>
> Changes since v2:
> * Improved inconsistent failure handling of clock rate setting
> * For completeness of usecase, added arc_emac_probe error handling
This doesn't apply to any of my trees.
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices
From: David Miller @ 2017-12-11 17:18 UTC (permalink / raw)
To: subashab; +Cc: netdev
In-Reply-To: <1512853110-4893-6-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Sat, 9 Dec 2017 13:58:29 -0700
> @@ -312,7 +321,8 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
>
> static size_t rmnet_get_size(const struct net_device *dev)
> {
> - return nla_total_size(2); /* IFLA_VLAN_ID */
> + return nla_total_size(2) /* IFLA_VLAN_ID */ +
> + nla_total_size(sizeof(struct ifla_vlan_flags)); /* IFLA_VLAN_FLAGS */
> }
>
Please indent this return statement properly, the second line of the
expression definitely should not start at the same column as the
'return'.
Thank you.
^ permalink raw reply
* Re: [PATCH V2] netlink: Add netns check on taps
From: Michal Kubecek @ 2017-12-11 17:22 UTC (permalink / raw)
To: David Miller
Cc: cernekee, johannes.berg, netdev, linux-kernel, daniel, dsahern
In-Reply-To: <20171211.115837.1416336431532133035.davem@davemloft.net>
On Mon, Dec 11, 2017 at 11:58:37AM -0500, David Miller wrote:
> From: Michal Kubecek <mkubecek@suse.cz>
> Date: Mon, 11 Dec 2017 17:13:50 +0100
>
> > On Wed, Dec 06, 2017 at 03:57:14PM -0500, David Miller wrote:
> >> From: Kevin Cernekee <cernekee@chromium.org>
> >> Date: Wed, 6 Dec 2017 12:12:27 -0800
> >>
> >> > Currently, a nlmon link inside a child namespace can observe systemwide
> >> > netlink activity. Filter the traffic so that nlmon can only sniff
> >> > netlink messages from its own netns.
> >> >
> >> > Test case:
> >> >
> >> > vpnns -- bash -c "ip link add nlmon0 type nlmon; \
> >> > ip link set nlmon0 up; \
> >> > tcpdump -i nlmon0 -q -w /tmp/nlmon.pcap -U" &
> >> > sudo ip xfrm state add src 10.1.1.1 dst 10.1.1.2 proto esp \
> >> > spi 0x1 mode transport \
> >> > auth sha1 0x6162633132330000000000000000000000000000 \
> >> > enc aes 0x00000000000000000000000000000000
> >> > grep --binary abc123 /tmp/nlmon.pcap
> >> >
> >> > Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> >>
> >> Applied and queued up for -stable, thanks Kevin.
> >
> > David,
> >
> > this patch is marked as accepted in patchworks and listed in
> >
> > http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
> >
> > but it's not in the net tree. Is there a problem with it?
>
> Sorry, it should be there now.
Thank you,
Michal Kubecek
^ permalink raw reply
* [PATCH ipsec net] xfrm: put policies when reusing pcpu xdst entry
From: Florian Westphal @ 2017-12-11 17:23 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
We need to put the policies when re-using the pcpu xdst entry, else
this leaks the reference.
Fixes: ec30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/xfrm/xfrm_policy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 9542975eb2f9..88143a480ec9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1833,6 +1833,7 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
sizeof(struct xfrm_policy *) * num_pols) == 0 &&
xfrm_xdst_can_reuse(xdst, xfrm, err)) {
dst_hold(&xdst->u.dst);
+ xfrm_pols_put(pols, num_pols);
while (err > 0)
xfrm_state_put(xfrm[--err]);
return xdst;
--
2.13.6
^ permalink raw reply related
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Randy Dunlap @ 2017-12-11 17:27 UTC (permalink / raw)
To: Daniel Borkmann, Richard Weinberger, netdev@vger.kernel.org
Cc: Linus Torvalds, Linux Kernel Mailing List, alexei.starovoitov,
brueckner
In-Reply-To: <ea01fc20-a122-e6d3-e147-2de7f9fa7e47@iogearbox.net>
On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>
> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
>> Randy,
>>
>> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
>>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
>>>> Another week, another rc.
>>>
>>> um (uml) won't build on i386 or x86_64:
>>>
>>> CC init/main.o
>>> In file included from ../include/linux/perf_event.h:18:0,
>>> from ../include/linux/trace_events.h:10,
>>> from ../include/trace/syscall.h:7,
>>> from ../include/linux/syscalls.h:82,
>>> from ../init/main.c:20:
>>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
>>> asm/bpf_perf_event.h: No such file or directory #include
>>> <asm/bpf_perf_event.h>
>>> ^
>>> compilation terminated.
>>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
>>
>> How do you trigger that build failure?
>> Can you share your .config?
Richard, it's just defconfig on both i386 and x86_64.
> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
> with uml, but looks like it's the only special case where there's no
> arch/um/include/uapi/asm/. What is the usual convention to pull in such
> headers in this case? Something like the below, would that fix it for you?
>
> Thanks for your help,
> Daniel
Yes, that patch works. Thanks.
> arch/um/include/asm/bpf_perf_event.h | 1 +
> include/asm-generic/bpf_perf_event.h | 1 +
> 2 files changed, 2 insertions(+)
> create mode 100644 arch/um/include/asm/bpf_perf_event.h
> create mode 100644 include/asm-generic/bpf_perf_event.h
>
> diff --git a/arch/um/include/asm/bpf_perf_event.h b/arch/um/include/asm/bpf_perf_event.h
> new file mode 100644
> index 0000000..3097758
> --- /dev/null
> +++ b/arch/um/include/asm/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <asm-generic/bpf_perf_event.h>
> diff --git a/include/asm-generic/bpf_perf_event.h b/include/asm-generic/bpf_perf_event.h
> new file mode 100644
> index 0000000..67112e5
> --- /dev/null
> +++ b/include/asm-generic/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <uapi/asm-generic/bpf_perf_event.h>
>
--
~Randy
^ permalink raw reply
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Daniel Borkmann @ 2017-12-11 17:32 UTC (permalink / raw)
To: Randy Dunlap, Richard Weinberger, netdev@vger.kernel.org
Cc: Linus Torvalds, Linux Kernel Mailing List, alexei.starovoitov,
brueckner
In-Reply-To: <96fb23e5-881d-46d5-b3ed-0a9269d4b831@infradead.org>
On 12/11/2017 06:27 PM, Randy Dunlap wrote:
> On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
>> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>>
>> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
>>> Randy,
>>>
>>> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
>>>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
>>>>> Another week, another rc.
>>>>
>>>> um (uml) won't build on i386 or x86_64:
>>>>
>>>> CC init/main.o
>>>> In file included from ../include/linux/perf_event.h:18:0,
>>>> from ../include/linux/trace_events.h:10,
>>>> from ../include/trace/syscall.h:7,
>>>> from ../include/linux/syscalls.h:82,
>>>> from ../init/main.c:20:
>>>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
>>>> asm/bpf_perf_event.h: No such file or directory #include
>>>> <asm/bpf_perf_event.h>
>>>> ^
>>>> compilation terminated.
>>>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
>>>
>>> How do you trigger that build failure?
>>> Can you share your .config?
>
> Richard, it's just defconfig on both i386 and x86_64.
>
>> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
>> with uml, but looks like it's the only special case where there's no
>> arch/um/include/uapi/asm/. What is the usual convention to pull in such
>> headers in this case? Something like the below, would that fix it for you?
>>
>> Thanks for your help,
>> Daniel
>
> Yes, that patch works. Thanks.
Thanks, I'd get it out later today, and route it via bpf tree.
^ permalink raw reply
* Re: [PATCH 1/1] rtlwifi: always initialize variables given to RT_TRACE()
From: Larry Finger @ 2017-12-11 17:34 UTC (permalink / raw)
To: Nicolas Iooss, Chaoming Li, Kalle Valo,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171210195159.7454-1-nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>
On 12/10/2017 01:51 PM, Nicolas Iooss wrote:
> In rtl_rx_ampdu_apply(), when rtlpriv->cfg->ops->get_btc_status()
> returns false, RT_TRACE() is called with the values of variables
> reject_agg and agg_size, which have not been initialized.
>
> Always initialize these variables in order to prevent using
> uninitialized values.
>
> This issue has been found with clang. The compiler reported:
>
> drivers/net/wireless/realtek/rtlwifi/base.c:1665:6: error: variable
> 'agg_size' is used uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
> if (rtlpriv->cfg->ops->get_btc_status())
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/base.c:1671:31: note:
> uninitialized use occurs here
> reject_agg, ctrl_agg_size, agg_size);
> ^~~~~~~~
>
> drivers/net/wireless/realtek/rtlwifi/base.c:1665:6: error: variable
> 'reject_agg' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (rtlpriv->cfg->ops->get_btc_status())
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/base.c:1671:4: note:
> uninitialized use occurs here
> reject_agg, ctrl_agg_size, agg_size);
> ^~~~~~~~~~
>
> Fixes: 2635664e6e4a ("rtlwifi: Add rx ampdu cfg for btcoexist.")
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>
Looks good. Acked-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Thanks,
Larry
> ---
> drivers/net/wireless/realtek/rtlwifi/base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
> index cad2272ae21b..704741d6f495 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/base.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/base.c
> @@ -1726,7 +1726,7 @@ int rtl_tx_agg_oper(struct ieee80211_hw *hw,
> void rtl_rx_ampdu_apply(struct rtl_priv *rtlpriv)
> {
> struct rtl_btc_ops *btc_ops = rtlpriv->btcoexist.btc_ops;
> - u8 reject_agg, ctrl_agg_size = 0, agg_size;
> + u8 reject_agg = 0, ctrl_agg_size = 0, agg_size = 0;
>
> if (rtlpriv->cfg->ops->get_btc_status())
> btc_ops->btc_get_ampdu_cfg(rtlpriv, &reject_agg,
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] tcp: do not overshoot window_clamp in tcp_rcv_space_adjust()
From: Neal Cardwell @ 2017-12-11 17:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-2-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
>
> While rcvbuf is properly clamped by tcp_rmem[2], rcvwin
> is left to a potentially too big value.
>
> It has no serious effect, since :
> 1) tcp_grow_window() has very strict checks.
> 2) window_clamp can be mangled by user space to any value anyway.
>
> tcp_init_buffer_space() and companions use tcp_full_space(),
> we use tcp_win_from_space() to avoid reloading sk->sk_rcvbuf
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [PATCH net-next 2/3] tcp: avoid integer overflows in tcp_rcv_space_adjust()
From: Neal Cardwell @ 2017-12-11 17:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-3-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
> When using large tcp_rmem[2] values (I did tests with 500 MB),
> I noticed overflows while computing rcvwin.
>
> Lets fix this before the following patch.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
From: Jiri Pirko @ 2017-12-11 17:59 UTC (permalink / raw)
To: David Miller; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211.120144.1060832843526341781.davem@davemloft.net>
Mon, Dec 11, 2017 at 06:01:44PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 11 Dec 2017 17:32:46 +0100
>
>> I think that it does not make sense to convert ethtool->netlink_ethtool
>> 1:1 feature wise. Now we have devlink, ritch switch representation
>> model, tc offload and many others. Lot of things that are in
>> ethtool, should be done in devlink. Also, there are couple of things
>> that should just die - nice example is ethtool --config-ntuple - we
>> should use tc for that.
>
>Whilst I do agree that devlink is probably a good place for this stuff
>(we want to be able to do ethetool things on objects that lack a netdev)
>I do not agree with the tc angle.
>
>It is entirely appropriate to set the ntuple settings of a driver
>without being required to use TC or similar.
>
>All you are going to do with your suggestion is make people keep using
>the existing ethtool ioctl, because they'll say "screw this, I'm not
>using TC I have something which works just fine already". And that's
>not the goal of putting this stuff into netlink, we want people to
>use the new facilities and move off of the ioctl.
Sure, but this is a great opportunity to avoid copying old mistakes.
That is why I suggested to do it not 1:1 but rather introduce brand new
netlink-based interface that would not carry old baggage.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: smoother receiver autotuning
From: Neal Cardwell @ 2017-12-11 17:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-4-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
> Back in linux-3.13 (commit b0983d3c9b13 ("tcp: fix dynamic right sizing"))
> I addressed the pressing issues we had with receiver autotuning.
>
> But DRS suffers from extra latencies caused by rcv_rtt_est.rtt_us
> drifts. One common problem happens during slow start, since the
> apparent RTT measured by the receiver can be inflated by ~50%,
> at the end of one packet train.
>
> Also, a single drop can delay read() calls by one RTT, meaning
> tcp_rcv_space_adjust() can be called one RTT too late.
>
> By replacing the tri-modal heuristic with a continuous function,
> we can offset the effects of not growing 'at the optimal time'.
>
> The curve of the function matches prior behavior if the space
> increased by 25% and 50% exactly.
>
> Cost of added multiply/divide is small, considering a TCP flow
> typically would run this part of the code few times in its life.
>
> I tested this patch with 100 ms RTT / 1% loss link, 100 runs
> of (netperf -l 5), and got an average throughput of 4600 Mbit
> instead of 1700 Mbit.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface
From: Jiri Pirko @ 2017-12-11 18:02 UTC (permalink / raw)
To: David Miller; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211.115651.1046181633998981619.davem@davemloft.net>
Mon, Dec 11, 2017 at 05:56:51PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 11 Dec 2017 17:02:21 +0100
>
>> Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@suse.cz wrote:
>>>No function implemented yet, only genetlink and module infrastructure.
>>>Register/unregister genetlink family "ethtool" and allow the module to be
>>>autoloaded by genetlink code (if built as a module, distributions would
>>>probably prefer "y").
>>>
>>>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>>
>> [...]
>>
>>
>>>+
>>>+/* identifies the device to query/set
>>>+ * - use either ifindex or ifname, not both
>>>+ * - for dumps and messages not related to a particular devices, fill neither
>>>+ * - info_mask is a bitfield, interpretation depends on the command
>>>+ */
>>>+struct ethnlmsghdr {
>>>+ __u32 ifindex; /* device ifindex */
>>>+ __u16 flags; /* request/response flags */
>>>+ __u16 info_mask; /* request/response info mask */
>>>+ char ifname[IFNAMSIZ]; /* device name */
>>
>> Why do you need this header? You can have 2 attrs:
>> ETHTOOL_ATTR_IFINDEX and
>> ETHTOOL_ATTR_IFNAME
>>
>> Why do you need per-command flags and info_mask? Could be bitfield
>> attr if needed by specific command.
>
>Jiri, we've had this discussion before :-)
>
>For elements which are common to most, if not all, requests it makes
>sense to define a base netlink message.
>
>My opinion on this matter has not changed at all since the last time
>we discussed this.
The discussion we had before was about flag bitfield that was there
*always*. In this case, that is not true. It is either ifindex or
ifname. Even rtnetlink has ifname as attribute.
The flags and info_mask is just big mystery. If it is per-command,
seems natural to have it as attributes.
>
>So unless you have new information to provide to me on this issue
>which might change my mind, please accept the result of the previous
>discussion which is that a base netlink message is not only
>appropriate but desirable.
>
>Thank you.
^ permalink raw reply
* [net 1/1] tipc: eliminate potential memory leak
From: Jon Maloy @ 2017-12-11 18:11 UTC (permalink / raw)
To: davem, netdev
Cc: tipc-discussion, hoang.h.le, mohan.krishna.ghanta.krishnamurthy
In the function tipc_sk_mcast_rcv() we call refcount_dec(&skb->users)
on received sk_buffers. Since the reference counter might hit zero at
this point, we have a potential memory leak.
We fix this by replacing refcount_dec() with kfree_skb().
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 5d18c0c..41127d0 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1140,7 +1140,7 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
__skb_dequeue(arrvq);
__skb_queue_tail(inputq, skb);
}
- refcount_dec(&skb->users);
+ kfree_skb(skb);
spin_unlock_bh(&inputq->lock);
continue;
}
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Richard Weinberger @ 2017-12-11 18:19 UTC (permalink / raw)
To: Randy Dunlap
Cc: Daniel Borkmann, netdev@vger.kernel.org, Linus Torvalds,
Linux Kernel Mailing List, alexei.starovoitov, brueckner
In-Reply-To: <96fb23e5-881d-46d5-b3ed-0a9269d4b831@infradead.org>
Am Montag, 11. Dezember 2017, 18:27:40 CET schrieb Randy Dunlap:
> On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
> > Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
> >
> > On 12/11/2017 09:32 AM, Richard Weinberger wrote:
> >> Randy,
> >>
> >> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
> >>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
> >>>> Another week, another rc.
> >>>
> >>> um (uml) won't build on i386 or x86_64:
> >>> CC init/main.o
> >>>
> >>> In file included from ../include/linux/perf_event.h:18:0,
> >>>
> >>> from ../include/linux/trace_events.h:10,
> >>> from ../include/trace/syscall.h:7,
> >>> from ../include/linux/syscalls.h:82,
> >>>
> >>> from ../init/main.c:20:
> >>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
> >>> asm/bpf_perf_event.h: No such file or directory #include
> >>> <asm/bpf_perf_event.h>
> >>>
> >>> ^
> >>>
> >>> compilation terminated.
> >>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
> >>
> >> How do you trigger that build failure?
> >> Can you share your .config?
>
> Richard, it's just defconfig on both i386 and x86_64.
How odd, here it used to build.
...until I did a make mrproper, and tried again. ;-\
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Neil Horman @ 2017-12-11 18:28 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Xin Long, network dev, linux-sctp, davem, syzkaller
In-Reply-To: <20171211153639.GA3532@localhost.localdomain>
On Mon, Dec 11, 2017 at 01:36:39PM -0200, Marcelo Ricardo Leitner wrote:
> Hi,
>
> On Mon, Dec 11, 2017 at 09:54:34AM -0500, Neil Horman wrote:
> > On Sun, Dec 10, 2017 at 03:40:51PM +0800, Xin Long wrote:
> > > Now in sctp_setsockopt_reset_streams, it only does the check
> > > optlen < sizeof(*params) for optlen. But it's not enough, as
> > > params->srs_number_streams should also match optlen.
> > >
> > > If the streams in params->srs_stream_list are less than stream
> > > nums in params->srs_number_streams, later when dereferencing
> > > the stream list, it could cause a slab-out-of-bounds crash, as
> > > reported by syzbot.
> > >
> > > This patch is to fix it by also checking the stream numbers in
> > > sctp_setsockopt_reset_streams to make sure at least it's not
> > > greater than the streams in the list.
> > >
> > > Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
> > > Reported-by: Dmitry Vyukov <dvyukov@google.com>
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > > net/sctp/socket.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > index 014847e..dbf140d 100644
> > > --- a/net/sctp/socket.c
> > > +++ b/net/sctp/socket.c
> > > @@ -3891,13 +3891,17 @@ static int sctp_setsockopt_reset_streams(struct sock *sk,
> > > struct sctp_association *asoc;
> > > int retval = -EINVAL;
> > >
> > > - if (optlen < sizeof(struct sctp_reset_streams))
> > > + if (optlen < sizeof(*params))
> > > return -EINVAL;
> > >
> > Is this going to work in all corner cases? IIRC struct
> > sctp_reset_stream has variable length array at the end of it, and so
> > sizeof(struct sctp_reset_streams) returns just the size of the
> > struct, while sizeof(*params) returns the size of the entire object
> > (including the array elements). If a user space task allocates a
>
> I don't think it can include the array elements as such information
> can't be passed from the application to the kernel other than via
> optlen parameter. There is no metadata around the struct that could
> allow that, and sizeof() is a constant, it can't be assuming different
> values in runtime.
>
> Cheers,
> Marcelo
>%
Yup, you're right, its fine.
Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > static memory block to hold this struct and the array, and passes it
> > in with a shorter optlen (if for example, they do not intend to use
> > all the array elements), this will cause a failure, where no failure
> > is truly warranted. It seems the correct check to me should be the
> > origional sizeof(struct sctp_reset_streams) check, and the below
> > check to ensure that there are at least the same number of array
> > elements available as indicated in srs_nuber_streams.
> >
> > Regards
> > Neil
> >
> > > params = memdup_user(optval, optlen);
> > > if (IS_ERR(params))
> > > return PTR_ERR(params);
> > >
> > > + if (params->srs_number_streams * sizeof(__u16) >
> > > + optlen - sizeof(*params))
> > > + goto out;
> > > +
> > > asoc = sctp_id2assoc(sk, params->srs_assoc_id);
> > > if (!asoc)
> > > goto out;
> > > --
> > > 2.1.0
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: correctly check return value of debugfs_create_dir
From: Jakub Kicinski @ 2017-12-11 18:40 UTC (permalink / raw)
To: Prashant Bhole; +Cc: 'David Miller', netdev
In-Reply-To: <002201d3723b$0e2b9310$2a82b930$@lab.ntt.co.jp>
On Mon, 11 Dec 2017 13:46:48 +0900, Prashant Bhole wrote:
> > From: David Miller [mailto:davem@davemloft.net]
> >
> > From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > Date: Fri, 8 Dec 2017 09:52:50 +0900
> >
> > > Return value is now checked with IS_ERROR_OR_NULL because
> > > debugfs_create_dir doesn't return error value. It either returns NULL
> > > or a valid pointer.
> > >
> > > Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > > ---
> > > drivers/net/netdevsim/netdev.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/netdevsim/netdev.c
> > > b/drivers/net/netdevsim/netdev.c index eb8c679fca9f..88d8ee2c89da
> > > 100644
> > > --- a/drivers/net/netdevsim/netdev.c
> > > +++ b/drivers/net/netdevsim/netdev.c
> > > @@ -469,7 +469,7 @@ static int __init nsim_module_init(void)
> > > int err;
> > >
> > > nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
> > > - if (IS_ERR(nsim_ddir))
> > > + if (IS_ERR_OR_NULL(nsim_ddir))
> > > return PTR_ERR(nsim_ddir);
> >
> > debugfs_create_dir() should really be fixed, either it uses error pointers
> > consistently and therefore always provides a suitable error code to return
> or it
> > always uses NULL.
> >
> > This in-between behavior makes using it as an interface painful because no
> clear
> > meaning is given to NULL.
> >
> > So please do the work necessary to make debugfs_create_dir()'s return
> > semantics clearer and more useful.
> >
> > Thank you.
>
> Dave,
> Thanks for comments. I will try to fix error handling in netdevsim first.
>
> Jakub,
> Let's decide with an example. The typical directory structure for netdevsim
> interface is as below:
> /sys/kernel/debug/netdevsim/sim0/bpf_bound_progs/
> Please let me know if you are ok with following:
>
> 1) If debugfs_create_dir() fails in module_init, let's keep it fatal error
> with corrected condition:
> + if (IS_ERR_OR_NULL(nsim_ddir))
> + return -ENOMEM;
>
> 2) In case sim0 or bpf_bound_progs are fail to create, we need to add
> checks before creating any file in them.
Fine with me, although if you fix DebugFS first you could use the real
error from the start here.
^ permalink raw reply
* Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface
From: David Miller @ 2017-12-11 18:45 UTC (permalink / raw)
To: jiri; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211180219.GB2047@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 11 Dec 2017 19:02:19 +0100
> The discussion we had before was about flag bitfield that was there
> *always*. In this case, that is not true. It is either ifindex or
> ifname. Even rtnetlink has ifname as attribute.
>
> The flags and info_mask is just big mystery. If it is per-command,
> seems natural to have it as attributes.
I think flags and info_mask indeed can be moved out of this struct.
I guess, in this case, I can see your point of view especially if we
allow ethtool operations on non-netdev entities.
So, ok, let's move forward without a base command struct and just
use attributes.
Thanks :)
^ permalink raw reply
* Re: [PATCH 1/3] PCI: introduce a device-managed version of pci_set_mwi
From: David Miller @ 2017-12-11 18:49 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, bhelgaas, netdev, linux-pci
In-Reply-To: <3e560e2b-7f11-74e2-6db5-60c8479dc59b@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Dec 2017 00:43:48 +0100
> Introduce a device-managed version of pci_set_mwi. First user is the
> Realtek r8169 driver.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Bjorn, can I get an ACK on this? And would you be OK with this going
through my tree as the patches later in this series give an example of
usage?
Thanks.
^ permalink raw reply
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Richard Weinberger @ 2017-12-11 18:49 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Randy Dunlap, netdev@vger.kernel.org, Linus Torvalds,
Linux Kernel Mailing List, alexei.starovoitov, brueckner
In-Reply-To: <ea01fc20-a122-e6d3-e147-2de7f9fa7e47@iogearbox.net>
Am Montag, 11. Dezember 2017, 11:19:54 CET schrieb Daniel Borkmann:
> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>
> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
> > Randy,
> >
> > Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
> >> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
> >>> Another week, another rc.
> >>
> >> um (uml) won't build on i386 or x86_64:
> >> CC init/main.o
> >>
> >> In file included from ../include/linux/perf_event.h:18:0,
> >>
> >> from ../include/linux/trace_events.h:10,
> >> from ../include/trace/syscall.h:7,
> >> from ../include/linux/syscalls.h:82,
> >>
> >> from ../init/main.c:20:
> >> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
> >> asm/bpf_perf_event.h: No such file or directory #include
> >> <asm/bpf_perf_event.h>
> >>
> >> ^
> >>
> >> compilation terminated.
> >> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
> >
> > How do you trigger that build failure?
> > Can you share your .config?
>
> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
> with uml, but looks like it's the only special case where there's no
> arch/um/include/uapi/asm/. What is the usual convention to pull in such
> headers in this case? Something like the below, would that fix it for you?
>
> Thanks for your help,
> Daniel
>
> arch/um/include/asm/bpf_perf_event.h | 1 +
> include/asm-generic/bpf_perf_event.h | 1 +
> 2 files changed, 2 insertions(+)
> create mode 100644 arch/um/include/asm/bpf_perf_event.h
> create mode 100644 include/asm-generic/bpf_perf_event.h
>
> diff --git a/arch/um/include/asm/bpf_perf_event.h
> b/arch/um/include/asm/bpf_perf_event.h new file mode 100644
> index 0000000..3097758
> --- /dev/null
> +++ b/arch/um/include/asm/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <asm-generic/bpf_perf_event.h>
> diff --git a/include/asm-generic/bpf_perf_event.h
> b/include/asm-generic/bpf_perf_event.h new file mode 100644
> index 0000000..67112e5
> --- /dev/null
> +++ b/include/asm-generic/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <uapi/asm-generic/bpf_perf_event.h>
Hmm, what about this?
diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 50a32c33d729..fb35ec000433 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -27,3 +27,4 @@ generic-y += trace_clock.h
generic-y += word-at-a-time.h
generic-y += xor.h
generic-y += kprobes.h
+generic-y += bpf_perf_event.h
--
sigma star gmbh - Eduard-Bodem-Gasse 6 - 6020 Innsbruck - Austria
ATU66964118 - FN 374287y
^ 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