* Re: [PATCH iproute2 3/3] bridge: request vlans along with link information
From: Roopa Prabhu @ 2017-09-10 5:26 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Roman Mashak, stephen@networkplumber.org, netdev@vger.kernel.org
In-Reply-To: <5706e1ba-6b85-4f32-a858-b5d87b17eb51@mojatatu.com>
On Sat, Sep 9, 2017 at 10:23 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 17-09-09 12:24 PM, Roopa Prabhu wrote:
>>
>> On Fri, Sep 8, 2017 at 2:52 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>>
>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>> ---
>>> bridge/link.c | 16 +++++++++++++---
>>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/bridge/link.c b/bridge/link.c
>>> index 60200f1..9e4206f 100644
>>> --- a/bridge/link.c
>>> +++ b/bridge/link.c
>>> @@ -461,9 +461,19 @@ static int brlink_show(int argc, char **argv)
>>> }
>>> }
>>>
>>> - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
>>> - perror("Cannon send dump request");
>>> - exit(1);
>>> + if (show_details) {
>>> + if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE,
>>> RTM_GETLINK,
>>> + (compress_vlans ?
>>> +
>>> RTEXT_FILTER_BRVLAN_COMPRESSED :
>>> + RTEXT_FILTER_BRVLAN)) < 0)
>>> {
>>> + perror("Cannon send dump request");
>>> + exit(1);
>>> + }
>>
>>
>> vlan information is already available with `bridge vlan show`. any
>> specific reason why you want it in
>> the link dump output ?
>>
>>
>>
>> The problem is this might just make the link dump larger and also add
>> too much clutter into the regular link dump output. iproute2 detailed
>> dump is already a bit hard to interpret. And without compression by
>> default, vlan info can just take over the link dump output. It will be
>> hard to look for other link attributes after that :). We deploy with
>> thousands of vlans and without compression even bridge vlan default
>> output is already hard to interpret.
>>
>
> Agree we should be turning on this stuff by default. i.e default stays
> compressed; otherwise it a huge dump.
>
> Having said that there is a lot of mess with this stuff.
> The bridge link events _already send this IFLA_AF_SPCE info_
> so not much choice there but to print it.
> At minimal we need that part because unfortunately there is no
> vlanfilter event in existence which will send us summaries of just
> vlans added to a port i.e both use XXXLINK.
> In general, the XXXLINK interface with these master devices (bridge,
> bond etc) continues to get messy. Recently started seeing events with
> devices claiming to be of KIND_slave etc on the bridge and bond devices;
yes, its a bit messy and redundant.
> yet at the same time my wireless card events are also showing up on the
> bridge link even though it is not enslaved there..
that is strange. It should not.
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Alexander Duyck @ 2017-09-10 1:28 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: Duyck, Alexander H, Netdev
In-Reply-To: <1504222032-6337-1-git-send-email-sridhar.samudrala@intel.com>
On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
<sridhar.samudrala@intel.com> wrote:
> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be used
> to enable symmetric tx and rx queues on a socket.
>
> This option is specifically useful for epoll based multi threaded workloads
> where each thread handles packets received on a single RX queue . In this model,
> we have noticed that it helps to send the packets on the same TX queue
> corresponding to the queue-pair associated with the RX queue specifically when
> busy poll is enabled with epoll().
>
> Two new fields are added to struct sock_common to cache the last rx ifindex and
> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns the cached
> rx queue when this option is enabled and the TX is happening on the same device.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> include/net/request_sock.h | 1 +
> include/net/sock.h | 17 +++++++++++++++++
> include/uapi/asm-generic/socket.h | 2 ++
> net/core/dev.c | 8 +++++++-
> net/core/sock.c | 10 ++++++++++
> net/ipv4/tcp_input.c | 1 +
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_minisocks.c | 1 +
> 8 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 23e2205..c3bc12e 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct request_sock *req)
> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
> sk_node_init(&req_to_sk(req)->sk_node);
> sk_tx_queue_clear(req_to_sk(req));
> + req_to_sk(req)->sk_symmetric_queues = sk_listener->sk_symmetric_queues;
> req->saved_syn = NULL;
> refcount_set(&req->rsk_refcnt, 0);
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 03a3625..3421809 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
> * @skc_node: main hash linkage for various protocol lookup tables
> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
> * @skc_tx_queue_mapping: tx queue number for this connection
> + * @skc_rx_queue_mapping: rx queue number for this connection
> + * @skc_rx_ifindex: rx ifindex for this connection
> * @skc_flags: place holder for sk_flags
> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
> * @skc_incoming_cpu: record/match cpu processing incoming packets
> * @skc_refcnt: reference count
> + * @skc_symmetric_queues: symmetric tx/rx queues
> *
> * This is the minimal network layer representation of sockets, the header
> * for struct sock and struct inet_timewait_sock.
> @@ -177,6 +180,7 @@ struct sock_common {
> unsigned char skc_reuseport:1;
> unsigned char skc_ipv6only:1;
> unsigned char skc_net_refcnt:1;
> + unsigned char skc_symmetric_queues:1;
> int skc_bound_dev_if;
> union {
> struct hlist_node skc_bind_node;
> @@ -214,6 +218,8 @@ struct sock_common {
> struct hlist_nulls_node skc_nulls_node;
> };
> int skc_tx_queue_mapping;
> + int skc_rx_queue_mapping;
> + int skc_rx_ifindex;
> union {
> int skc_incoming_cpu;
> u32 skc_rcv_wnd;
> @@ -324,6 +330,8 @@ struct sock {
> #define sk_nulls_node __sk_common.skc_nulls_node
> #define sk_refcnt __sk_common.skc_refcnt
> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>
> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
> @@ -340,6 +348,7 @@ struct sock {
> #define sk_reuseport __sk_common.skc_reuseport
> #define sk_ipv6only __sk_common.skc_ipv6only
> #define sk_net_refcnt __sk_common.skc_net_refcnt
> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
> #define sk_bind_node __sk_common.skc_bind_node
> #define sk_prot __sk_common.skc_prot
> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct sock *sk)
> return sk ? sk->sk_tx_queue_mapping : -1;
> }
>
> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff *skb)
> +{
> + if (sk->sk_symmetric_queues) {
> + sk->sk_rx_ifindex = skb->skb_iif;
> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
> + }
> +}
> +
> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
> {
> sk_tx_queue_clear(sk);
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index e47c9e4..f6b416e 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -106,4 +106,6 @@
>
> #define SO_ZEROCOPY 60
>
> +#define SO_SYMMETRIC_QUEUES 61
> +
> #endif /* __ASM_GENERIC_SOCKET_H */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 270b547..d96cda8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
>
> if (queue_index < 0 || skb->ooo_okay ||
> queue_index >= dev->real_num_tx_queues) {
> - int new_index = get_xps_queue(dev, skb);
> + int new_index = -1;
> +
> + if (sk && sk->sk_symmetric_queues && dev->ifindex == sk->sk_rx_ifindex)
> + new_index = sk->sk_rx_queue_mapping;
> +
> + if (new_index < 0 || new_index >= dev->real_num_tx_queues)
> + new_index = get_xps_queue(dev, skb);
>
> if (new_index < 0)
> new_index = skb_tx_hash(dev, skb);
So one thing I am not sure about is if we should be overriding XPS. It
might make sense to instead place this after XPS so that if the root
user configures it then it applies, otherwise if the socket is
requesting symmetric queues you could fall back to that, and then
finally just use hashing as the final solution for distributing the
workload.
That way if somebody decides to reserve queues for some sort of
specific traffic like AF_PACKET then they can configure the Tx via
XPS, configure the Rx via RSS redirection table reprogramming, and
then setup a filters on the hardware to direct the traffic they want
to the queues that are running AF_PACKET.
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 9b7b6bb..3876cce 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1059,6 +1059,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
> break;
>
> + case SO_SYMMETRIC_QUEUES:
> + sk->sk_symmetric_queues = valbool;
> + break;
> +
> default:
> ret = -ENOPROTOOPT;
> break;
> @@ -1391,6 +1395,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
> v.val = sock_flag(sk, SOCK_ZEROCOPY);
> break;
>
> + case SO_SYMMETRIC_QUEUES:
> + v.val = sk->sk_symmetric_queues;
> + break;
> +
> default:
> /* We implement the SO_SNDLOWAT etc to not be settable
> * (1003.1g 7).
> @@ -2738,6 +2746,8 @@ void sock_init_data(struct socket *sock, struct sock *sk)
> sk->sk_max_pacing_rate = ~0U;
> sk->sk_pacing_rate = ~0U;
> sk->sk_incoming_cpu = -1;
> + sk->sk_rx_ifindex = -1;
> + sk->sk_rx_queue_mapping = -1;
> /*
> * Before updating sk_refcnt, we must commit prior changes to memory
> * (Documentation/RCU/rculist_nulls.txt for details)
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index c5d7656..12381e0 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6356,6 +6356,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
> tcp_rsk(req)->snt_isn = isn;
> tcp_rsk(req)->txhash = net_tx_rndhash();
> tcp_openreq_init_rwin(req, sk, dst);
> + sk_mark_rx_queue(req_to_sk(req), skb);
> if (!want_cookie) {
> tcp_reqsk_record_syn(sk, req, skb);
> fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index a63486a..82f9af4 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1450,6 +1450,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>
> sock_rps_save_rxhash(sk, skb);
> sk_mark_napi_id(sk, skb);
> + sk_mark_rx_queue(sk, skb);
> if (dst) {
> if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
> !dst->ops->check(dst, 0)) {
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 188a6f3..2b5efd5 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -809,6 +809,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
>
> /* record NAPI ID of child */
> sk_mark_napi_id(child, skb);
> + sk_mark_rx_queue(child, skb);
>
> tcp_segs_in(tcp_sk(child), skb);
> if (!sock_owned_by_user(child)) {
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v2 net] net: qualcomm: rmnet: Fix a double free
From: David Miller @ 2017-09-09 21:34 UTC (permalink / raw)
To: dan.carpenter; +Cc: subashab, netdev, kernel-janitors
In-Reply-To: <20170909085803.zgfqhp23yt4ol3ka@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Sat, 9 Sep 2017 11:58:03 +0300
> There is a typo here so we accidentally free "skb" instead of "skbn".
> It leads to a double free and a leak. After discussing with Subash,
> it's better to just move the check before the allocation and avoid the
> need to free.
>
> Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Fix the leak as well. Thanks Subash!
Applied, thanks Dan.
^ permalink raw reply
* Re: [PATCH v2 net] net: qualcomm: rmnet: Fix a double free
From: Subash Abhinov Kasiviswanathan @ 2017-09-09 18:59 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, netdev, kernel-janitors
In-Reply-To: <20170909085803.zgfqhp23yt4ol3ka@mwanda>
On 2017-09-09 02:58, Dan Carpenter wrote:
> There is a typo here so we accidentally free "skb" instead of "skbn".
> It leads to a double free and a leak. After discussing with Subash,
> it's better to just move the check before the allocation and avoid the
> need to free.
>
> Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial
> implementation")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Fix the leak as well. Thanks Subash!
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> index 557c9bf1a469..86b8c758f94e 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -84,6 +84,10 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff
> *skb)
> if (((int)skb->len - (int)packet_len) < 0)
> return NULL;
>
> + /* Some hardware can send us empty frames. Catch them */
> + if (ntohs(maph->pkt_len) == 0)
> + return NULL;
> +
> skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
> if (!skbn)
> return NULL;
> @@ -94,11 +98,5 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff
> *skb)
> memcpy(skbn->data, skb->data, packet_len);
> skb_pull(skb, packet_len);
>
> - /* Some hardware can send us empty frames. Catch them */
> - if (ntohs(maph->pkt_len) == 0) {
> - kfree_skb(skb);
> - return NULL;
> - }
> -
> return skbn;
> }
Acked-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [patch net] net: sched: fix memleak for chain zero
From: Cong Wang @ 2017-09-09 18:46 UTC (permalink / raw)
To: David Miller
Cc: Jiri Pirko, Linux Kernel Network Developers, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
In-Reply-To: <20170907.191800.218886770521884093.davem@davemloft.net>
On Thu, Sep 7, 2017 at 7:18 PM, David Miller <davem@davemloft.net> wrote:
>
> This doesn't apply cleanly any more, please respin.
>
Sigh, you applied this patch despite of strong objections from me.
I seriously doubt your tastes now, David. Fine, this code does not
deserve a good taste at all, let bugs stay there.
Good luck!
^ permalink raw reply
* Re: [PATCH iproute2 3/3] bridge: request vlans along with link information
From: Nikolay Aleksandrov @ 2017-09-09 18:15 UTC (permalink / raw)
To: Jamal Hadi Salim, Roopa Prabhu, Roman Mashak
Cc: stephen@networkplumber.org, netdev@vger.kernel.org
In-Reply-To: <5706e1ba-6b85-4f32-a858-b5d87b17eb51@mojatatu.com>
On 09/09/17 20:23, Jamal Hadi Salim wrote:
> On 17-09-09 12:24 PM, Roopa Prabhu wrote:
>> On Fri, Sep 8, 2017 at 2:52 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>> ---
>>> bridge/link.c | 16 +++++++++++++---
>>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/bridge/link.c b/bridge/link.c
>>> index 60200f1..9e4206f 100644
>>> --- a/bridge/link.c
>>> +++ b/bridge/link.c
>>> @@ -461,9 +461,19 @@ static int brlink_show(int argc, char **argv)
>>> }
>>> }
>>>
>>> - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
>>> - perror("Cannon send dump request");
>>> - exit(1);
>>> + if (show_details) {
>>> + if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
>>> + (compress_vlans ?
>>> + RTEXT_FILTER_BRVLAN_COMPRESSED :
>>> + RTEXT_FILTER_BRVLAN)) < 0) {
>>> + perror("Cannon send dump request");
>>> + exit(1);
>>> + }
>>
>> vlan information is already available with `bridge vlan show`. any
>> specific reason why you want it in
>> the link dump output ?
>>
>>
>> The problem is this might just make the link dump larger and also add
>> too much clutter into the regular link dump output. iproute2 detailed
>> dump is already a bit hard to interpret. And without compression by
>> default, vlan info can just take over the link dump output. It will be
>> hard to look for other link attributes after that :). We deploy with
>> thousands of vlans and without compression even bridge vlan default
>> output is already hard to interpret.
>>
>
> Agree we should be turning on this stuff by default. i.e default stays
> compressed; otherwise it a huge dump.
I think this should be dumped with the getlink request only on some additional
flag. The getlink does not include these by default.
>
> Having said that there is a lot of mess with this stuff.
> The bridge link events _already send this IFLA_AF_SPCE info_
> so not much choice there but to print it.
Right, on NEWLINK per port notification you'll get the compressed vlan info.
> At minimal we need that part because unfortunately there is no
> vlanfilter event in existence which will send us summaries of just
> vlans added to a port i.e both use XXXLINK.
But let's either add a new flag or use -compressvlans to print it when monitoring/showing
link otherwise people who are monitoring only the port flags will start getting lists
with vlans. Even compressed these can still be quite long and confusing, especially
when monitoring.
> In general, the XXXLINK interface with these master devices (bridge,
> bond etc) continues to get messy. Recently started seeing events with
> devices claiming to be of KIND_slave etc on the bridge and bond devices;
> yet at the same time my wireless card events are also showing up on the
> bridge link even though it is not enslaved there..
>
> cheers,
> jamal
>
>> Can you please paste a sample default detailed output with your patch
>> with a few hundred vlans ?
>>
>
^ permalink raw reply
* Re: [PATCH iproute2 3/3] bridge: request vlans along with link information
From: Jamal Hadi Salim @ 2017-09-09 17:23 UTC (permalink / raw)
To: Roopa Prabhu, Roman Mashak
Cc: stephen@networkplumber.org, netdev@vger.kernel.org
In-Reply-To: <CAJieiUhT75fM3LuhNW32QkbJA9TO5dHwb_G-aRD2H70jF-FG2A@mail.gmail.com>
On 17-09-09 12:24 PM, Roopa Prabhu wrote:
> On Fri, Sep 8, 2017 at 2:52 PM, Roman Mashak <mrv@mojatatu.com> wrote:
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>> bridge/link.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/bridge/link.c b/bridge/link.c
>> index 60200f1..9e4206f 100644
>> --- a/bridge/link.c
>> +++ b/bridge/link.c
>> @@ -461,9 +461,19 @@ static int brlink_show(int argc, char **argv)
>> }
>> }
>>
>> - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
>> - perror("Cannon send dump request");
>> - exit(1);
>> + if (show_details) {
>> + if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
>> + (compress_vlans ?
>> + RTEXT_FILTER_BRVLAN_COMPRESSED :
>> + RTEXT_FILTER_BRVLAN)) < 0) {
>> + perror("Cannon send dump request");
>> + exit(1);
>> + }
>
> vlan information is already available with `bridge vlan show`. any
> specific reason why you want it in
> the link dump output ?
>
>
> The problem is this might just make the link dump larger and also add
> too much clutter into the regular link dump output. iproute2 detailed
> dump is already a bit hard to interpret. And without compression by
> default, vlan info can just take over the link dump output. It will be
> hard to look for other link attributes after that :). We deploy with
> thousands of vlans and without compression even bridge vlan default
> output is already hard to interpret.
>
Agree we should be turning on this stuff by default. i.e default stays
compressed; otherwise it a huge dump.
Having said that there is a lot of mess with this stuff.
The bridge link events _already send this IFLA_AF_SPCE info_
so not much choice there but to print it.
At minimal we need that part because unfortunately there is no
vlanfilter event in existence which will send us summaries of just
vlans added to a port i.e both use XXXLINK.
In general, the XXXLINK interface with these master devices (bridge,
bond etc) continues to get messy. Recently started seeing events with
devices claiming to be of KIND_slave etc on the bridge and bond devices;
yet at the same time my wireless card events are also showing up on the
bridge link even though it is not enslaved there..
cheers,
jamal
> Can you please paste a sample default detailed output with your patch
> with a few hundred vlans ?
>
^ permalink raw reply
* Re: [PATCH v1 3/3] net: skb_queue_purge(): lock/unlock the queue only once
From: Eric Dumazet @ 2017-09-09 16:52 UTC (permalink / raw)
To: Michael Witten
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Stephen Hemminger, netdev, linux-kernel
In-Reply-To: <0c67e720495b4c1a920ed0f7a7e1daeb-mfwitten@gmail.com>
On Sat, 2017-09-09 at 05:50 +0000, Michael Witten wrote:
> Thanks for your input, Eric Dumazet and Stephen Hemminger; based on
> your observations, this version of the patch implements a very
> lightweight purging of the queue.
net-next is closed.
Documentation/networking/netdev-FAQ.txt
Meaning we are chasing bugs at this moment, not adding new ones.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver
From: Florian Fainelli @ 2017-09-09 16:30 UTC (permalink / raw)
To: Kunihiko Hayashi, netdev, David S. Miller, Andrew Lunn
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
devicetree, Masahiro Yamada, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1504875731-3680-3-git-send-email-hayashi.kunihiko@socionext.com>
On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> The UniPhier platform from Socionext provides the AVE ethernet
> controller that includes MAC and MDIO bus supporting RGMII/RMII
> modes. The controller is named AVE.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
[snip]
> +static int ave_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + u32 proc_idx, done_idx, ndesc, cmdsts;
> + int freepkt;
> + unsigned char *buffptr = NULL; /* buffptr for descriptor */
> + unsigned int len;
> + dma_addr_t paddr;
> +
> + proc_idx = priv->tx.proc_idx;
> + done_idx = priv->tx.done_idx;
> + ndesc = priv->tx.ndesc;
> + freepkt = ((done_idx + ndesc - 1) - proc_idx) % ndesc;
> +
> + /* not enough entry, then we stop queue */
> + if (unlikely(freepkt < 2)) {
> + netif_stop_queue(ndev);
> + if (unlikely(freepkt < 1))
> + return NETDEV_TX_BUSY;
This looks wrong, why are you checking first for less than 2
descriptors, and if there is none, NETDEV_TX_BUSY? If you need 2 slots
to complete a transmision, stop the transmit queue and return
NETDEV_TX_BUSY.
> + }
> +
> + priv->tx.desc[proc_idx].skbs = skb;
> +
> + /* add padding for short packet */
> + if (skb_padto(skb, ETH_ZLEN)) {
> + dev_kfree_skb_any(skb);
> + return NETDEV_TX_OK;
> + }
skb_padto() frees the SKB in case of error, that would lead to a double
free here.
> +
> + buffptr = skb->data - NET_IP_ALIGN;
> + len = max_t(unsigned int, ETH_ZLEN, skb->len);
If you use skb_put_padto() if padding was necessary skb->len will be at
least ETH_ZLEN, so you can remove this.
> +
> + paddr = ave_dma_map(ndev, &priv->tx.desc[proc_idx], buffptr,
> + len + NET_IP_ALIGN, DMA_TO_DEVICE);
As mentioned before you can't assume this will never fail.
> + paddr += NET_IP_ALIGN;
> +
> + /* set buffer address to descriptor */
> + ave_wdesc_addr(ndev, AVE_DESCID_TX, proc_idx, 4, paddr);
Also mentioned in the other email, make this 4 a constant so we know
it's an offset and not a length.
> +
> + /* set flag and length to send */
> + cmdsts = AVE_STS_OWN | AVE_STS_1ST | AVE_STS_LAST
> + | (len & AVE_STS_PKTLEN_TX);
AVE_STS_PKTLEN_TX would be better named with a _MASK suffix.
> +
> + /* set interrupt per AVE_FORCE_TXINTCNT or when queue is stopped */
> + if (!(proc_idx % AVE_FORCE_TXINTCNT) || netif_queue_stopped(ndev))
> + cmdsts |= AVE_STS_INTR;
> +
> + /* disable checksum calculation when skb doesn't calurate checksum */
> + if (skb->ip_summed == CHECKSUM_NONE ||
> + skb->ip_summed == CHECKSUM_UNNECESSARY)
> + cmdsts |= AVE_STS_NOCSUM;
> +
> + /* set cmdsts */
> + ave_wdesc(ndev, AVE_DESCID_TX, proc_idx, 0, cmdsts);
> +
> + priv->tx.proc_idx = (proc_idx + 1) % ndesc;
You should also check the ring space after transmission and assert flow
control on the transmit queue if needed.
> +
> + return NETDEV_TX_OK;
> +}
[snip]
> +static struct net_device_stats *ave_stats(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + u32 drop_num = 0;
> +
> + priv->stats.rx_errors = ave_r32(ndev, AVE_BFCR);
> +
> + drop_num += ave_r32(ndev, AVE_RX0OVFFC);
> + drop_num += ave_r32(ndev, AVE_SN5FC);
> + drop_num += ave_r32(ndev, AVE_SN6FC);
> + drop_num += ave_r32(ndev, AVE_SN7FC);
> + priv->stats.rx_dropped = drop_num;
> +
You should consider switching to 64-bit statistics, this requires a
little bit more work for 32-bit hosts (see
include/linux/u64_stats_sync.h) but this allows you to keep statistics
around above 4GB.
> + return &priv->stats;
> +}
> +--
Florian
^ permalink raw reply
* Re: [PATCH iproute2 3/3] bridge: request vlans along with link information
From: Roopa Prabhu @ 2017-09-09 16:24 UTC (permalink / raw)
To: Roman Mashak
Cc: stephen@networkplumber.org, netdev@vger.kernel.org,
Jamal Hadi Salim
In-Reply-To: <1504907543-14394-4-git-send-email-mrv@mojatatu.com>
On Fri, Sep 8, 2017 at 2:52 PM, Roman Mashak <mrv@mojatatu.com> wrote:
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> bridge/link.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/bridge/link.c b/bridge/link.c
> index 60200f1..9e4206f 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -461,9 +461,19 @@ static int brlink_show(int argc, char **argv)
> }
> }
>
> - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
> - perror("Cannon send dump request");
> - exit(1);
> + if (show_details) {
> + if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
> + (compress_vlans ?
> + RTEXT_FILTER_BRVLAN_COMPRESSED :
> + RTEXT_FILTER_BRVLAN)) < 0) {
> + perror("Cannon send dump request");
> + exit(1);
> + }
vlan information is already available with `bridge vlan show`. any
specific reason why you want it in
the link dump output ?
The problem is this might just make the link dump larger and also add
too much clutter into the regular link dump output. iproute2 detailed
dump is already a bit hard to interpret. And without compression by
default, vlan info can just take over the link dump output. It will be
hard to look for other link attributes after that :). We deploy with
thousands of vlans and without compression even bridge vlan default
output is already hard to interpret.
Can you please paste a sample default detailed output with your patch
with a few hundred vlans ?
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions
From: Andrew Lunn @ 2017-09-09 15:55 UTC (permalink / raw)
To: Jassi Brar
Cc: Florian Fainelli, Kunihiko Hayashi, netdev, David S. Miller,
Rob Herring, Mark Rutland, linux-arm-kernel@lists.infradead.org,
lkml, Devicetree List, Masahiro Yamada, Masami Hiramatsu,
Jongsung Kim
In-Reply-To: <CAJe_ZhciWBhw5npxEjTUGw2duS7DSfhzP5nzAOy_ig4-1XcPWw@mail.gmail.com>
On Sat, Sep 09, 2017 at 09:03:05AM +0530, Jassi Brar wrote:
> On 9 September 2017 at 00:21, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> >> From: Jassi Brar <jaswinder.singh@linaro.org>
> >>
> >> Add RTL8201F phy-id and the related functions to the driver.
> >>
> >> The original patch is as follows:
> >> https://patchwork.kernel.org/patch/2538341/
> >>
> >> Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
> >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >> ---
> >> drivers/net/phy/realtek.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 45 insertions(+)
> >>
> >> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> >> index 9cbe645..d9974ce 100644
> >> --- a/drivers/net/phy/realtek.c
> >> +++ b/drivers/net/phy/realtek.c
> >> @@ -29,10 +29,23 @@
> >> #define RTL8211F_PAGE_SELECT 0x1f
> >> #define RTL8211F_TX_DELAY 0x100
> >>
> >> +#define RTL8201F_ISR 0x1e
> >> +#define RTL8201F_PAGE_SELECT 0x1f
> >
> > We have a page select register define for the RTL8211F right above, so
> > surely we can make that a common definition?
> >
> That is just for the sake of consistency.
> I mean RTL8211 wouldn't look neat among everything else RTL8201.
>
> Also the page-select offsets just _happen_ to be same value...
If you look at all the other supported PHYs, they all consistently use
the same page register across models. Marvell is always 22, mscc is
always 31, vitesse is always 31.
I would say it is a safe bet that all realtek PHYs will use 0x1f for
page select. So please add a patch which renames RTL8211F_PAGE_SELECT
to RTL821x_PAGE_SELECT.
It is best to do this now. I spent a while cleaning up the mess the
Marvell driver had got into with its page select code. Lots of
duplicate code and defines doing the same thing.
Andrew
^ permalink raw reply
* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Andrew Lunn @ 2017-09-09 15:45 UTC (permalink / raw)
To: Tristram.Ha
Cc: pavel, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41123010@CHN-SV-EXMX02.mchp-main.com>
On Sat, Sep 09, 2017 at 01:44:37AM +0000, Tristram.Ha@microchip.com wrote:
> > -----Original Message-----
> > From: Pavel Machek [mailto:pavel@ucw.cz]
> > Sent: Friday, September 08, 2017 2:58 PM
> > To: Tristram Ha - C24268
> > Cc: andrew@lunn.ch; muvarov@gmail.com; nathan.leigh.conrad@gmail.com;
> > vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh -
> > C21699
> > Subject: Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
> >
> > Hi!
> >
> > > > > + default:
> > > > > + processed = false;
> > > > > + break;
> > > > > + }
> > > > > + if (processed)
> > > > > + *val = data;
> > > > > +}
> > > >
> > > > Similar code will be needed by other drivers, right?
> > >
> > > Although KSZ8795 and KSZ8895 may use the same code, the other
> > > chips will have different code.
> >
> > Ok, please make sure code is shared between these two.
>
> The exact function probably cannot be shared between KSZ8795
> and KSZ8895 drivers because although the register constants look
> the same but their exact locations are different in the 2 chips.
You need to be careful with such functions. If they look identical,
somebody will try to consolidate them into one. So either you need to
do the consolidation yourself in order to get it right, or you need to
add a comment about how it differs.
Andrew
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-09 11:28 UTC (permalink / raw)
To: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp>
On 07/09/17 01:47, Kosuke Tatsukawa wrote:
> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
> balance-alb mode") tried to fix transmit dynamic load balancing in
> balance-alb mode, which wasn't working after commit 8b426dc54cf4
> ("bonding: remove hardcoded value").
>
> It turned out that my previous patch only fixed the case when
> balance-alb was specified as bonding module parameter, and not when
> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
> common usage). In the latter case, tlb_dynamic_lb was set up according
> to the default mode of the bonding interface, which happens to be
> balance-rr.
>
> This additional patch addresses this issue by setting up tlb_dynamic_lb
> to 1 if "mode" is set to balance-alb through the sysfs interface.
>
> I didn't add code to change tlb_balance_lb back to the default value for
> other modes, because "mode" is usually set up only once during
> initialization, and it's not worthwhile to change the static variable
> bonding_defaults in bond_main.c to a global variable just for this
> purpose.
>
> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
> balance-tlb mode if it is set up using the sysfs interface. I didn't
> change that behavior, because the value of tlb_balance_lb can be changed
> using the sysfs interface for balance-tlb, and I didn't like changing
> the default value back and forth for balance-tlb.
>
> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
> is not an intended usage, so there is little use making it writable at
> this moment.
>
> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
> Reported-by: Reinis Rozitis <r@roze.lv>
> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
> Cc: stable@vger.kernel.org # v4.12+
> ---
> drivers/net/bonding/bond_options.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
This fix is simpler and more suitable for -net, it fixes the case where
we switch to ALB mode with tlb_dynamic_lb = 0. After it is in I'll fix the
default tlb_dynamic_lb issue and restore the original behaviour.
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-09 11:23 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <9ae82441-78da-0056-88c1-2f9d863fb81f@cumulusnetworks.com>
On 09/09/17 13:29, Nikolay Aleksandrov wrote:
> On 09/09/17 02:54, Mahesh Bandewar (महेश बंडेवार) wrote:
>> On Fri, Sep 8, 2017 at 7:30 AM, Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com> wrote:
>>> On 08/09/17 17:17, Kosuke Tatsukawa wrote:
[snip]
>>>
>> I think the underlying issue is that tlb_dynamic_lb should be set to 1
>> for all modes which was not the case when it was getting initialized
>> only forTLB mode. So from that perspective I prefer Nik's patch with a
>> small variation that guards the case when mode transitions from TLB to
>> ALB. The reason why I like that patch is because it's simple and
>> avoids complications.
>
> +1, I think this is the most straight-forward solution as well and
> safest for -net
>
> I will go ahead and submit it in a few minutes.
>
> Thanks,
> Nik
>
Just FYI, since the second fix (tlb_dynamic_lb in TLB = 0 switch to ALB) is identical
to this patch, I'm acking this one and will wait until it's in to submit the default
value fix (if we start in non-TLB mode and switch to TLB we'll get dynamic_lb = 0
currently).
I guess this is the simplest way, I didn't want to alter user-configured value on
mode switch, but it seems better than the alternative especially for -net.
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-09 10:29 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <CAF2d9jgHLJ28hpiSafL7823-Ga4y8aYZ2t_Sqh7D51VL_DgDww@mail.gmail.com>
On 09/09/17 02:54, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Fri, Sep 8, 2017 at 7:30 AM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> On 08/09/17 17:17, Kosuke Tatsukawa wrote:
>>> Hi,
>>>
>>>> On 08/09/17 13:10, Nikolay Aleksandrov wrote:
>>>>> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
>>>>>> Hi,
>>>>>>
>>>>>>> On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>>>>>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>>>>>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>>>>>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>>>>>>> ("bonding: remove hardcoded value").
>>>>>>>>
>>>>>>>> It turned out that my previous patch only fixed the case when
>>>>>>>> balance-alb was specified as bonding module parameter, and not when
>>>>>>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>>>>>>>> common usage). In the latter case, tlb_dynamic_lb was set up according
>>>>>>>> to the default mode of the bonding interface, which happens to be
>>>>>>>> balance-rr.
>>>>>>>>
>>>>>>>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>>>>>>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>>>>>>
>>>>>>>> I didn't add code to change tlb_balance_lb back to the default value for
>>>>>>>> other modes, because "mode" is usually set up only once during
>>>>>>>> initialization, and it's not worthwhile to change the static variable
>>>>>>>> bonding_defaults in bond_main.c to a global variable just for this
>>>>>>>> purpose.
>>>>>>>>
>>>>>>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>>>>>>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>>>>>>>> change that behavior, because the value of tlb_balance_lb can be changed
>>>>>>>> using the sysfs interface for balance-tlb, and I didn't like changing
>>>>>>>> the default value back and forth for balance-tlb.
>>>>>>>>
>>>>>>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>>>>>>>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>>>>>>>> is not an intended usage, so there is little use making it writable at
>>>>>>>> this moment.
>>>>>>>>
>>>>>>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>>>>>>>> Reported-by: Reinis Rozitis <r@roze.lv>
>>>>>>>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>>>>>>>> Cc: stable@vger.kernel.org # v4.12+
>>>>>>>> ---
>>>>>>>> drivers/net/bonding/bond_options.c | 3 +++
>>>>>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>> I don't believe this to be the right solution, hardcoding it like this
>>>>>>> changes user-visible behaviour. The issue is that if someone configured
>>>>>>> it to be 0 in tlb mode, suddenly it will become 1 and will silently
>>>>>>> override their config if they switch the mode to alb. Also it robs users
>>>>>>> from their choice.
>>>>>>>
>>>>>>> If you think this should be settable in ALB mode, then IMO you should
>>>>>>> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
>>>>>>> That would also be consistent with how it's handled in TLB mode.
>>>>>>
>>>>>> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
>>>>>> this point. All the current commits regarding tlb_dynamic_lb are for
>>>>>> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
>>>>>> to 0 is an intended usage.
>>>>>>
>>>>>>
>>>>>>> Going back and looking at your previous fix I'd argue that it is also
>>>>>>> wrong, you should've removed the mode check altogether to return the
>>>>>>> original behaviour where the dynamic_lb is set to 1 (enabled) by
>>>>>>> default and then ALB mode would've had it, of course that would've left
>>>>>>> the case of setting it to 0 in TLB mode and switching to ALB, but that
>>>>>>> is a different issue.
>>>>>>
>>>>>> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
>>>>>> tlb_dynamic_lb is referenced in the following functions.
>>>>>>
>>>>>> + bond_do_alb_xmit() -- Used by both balance-tlb and balance-alb
>>>>>> + bond_tlb_xmit() -- Only used by balance-tlb
>>>>>> + bond_open() -- Used by both balance-tlb and balance-alb
>>>>>> + bond_check_params() -- Used during module initialization
>>>>>> + bond_fill_info() -- Used to get/set value
>>>>>> + bond_option_tlb_dynamic_lb_set() -- Used to get/set value
>>>>>> + bonding_show_tlb_dynamic_lb() -- Used to get/set value
>>>>>> + bond_is_nondyn_tlb() -- Only referenced if balance-tlb mode
>>>>>>
>>>>>> The following untested patch adds code to make balance-alb work as if
>>>>>> tlb_dynamic_lb==1 for the functions which affect balance-alb mode. It
>>>>>> also reverts my previous patch.
>>>>>>
>>>>>> What do you think about this approach?
>>>>>> ---
>>>>>> Kosuke TATSUKAWA | 1st Platform Software Division
>>>>>> | NEC Solution Innovators
>>>>>> | tatsu@ab.jp.nec.com
>>>>>>
>>>>>
>>>>> Logically the approach looks good, that being said it adds unnecessary tests in
>>>>> the fast path, why not just something like the patch below ? That only leaves the
>>>>> problem if it is zeroed in TLB and switched to ALB mode, and that is a one line
>>>>> fix to unsuppmodes just allow it to be set for that specific case. The below
>>>>> returns the default behaviour before the commit in your Fixes tag.
>>>>>
>>>>>
>>>>
>>>> Actually I'm fine with your approach, too. It will fix this regardless of the
>>>> value of tlb_dynamic_lb which sounds good to me for the price of a test in
>>>> the fast path.
>>>
>>> If you're concerned about the additional test in the fast path, how
>>> about the patch below. I've added an arguemnt to bond_do_alb_xmit()
>>> to handle both balance-tlb and balance-alb similary.
>>>
>>
>> Even better, looks great! 1 question below though.
>>
>>> I'm not sure if this causes any problem if tlb_dynamic_lb is changed
>>> while calling bond_do_alb_xmit() in balance-tlb mode.
>>
>> The option has the ifdown flag, you shouldn't be able to change it while
>> the bond dev is up, but even if you could I don't think it will be an issue
>> for the xmit.
>>
>>> ---
>>> Kosuke TATSUKAWA | 1st Platform Software Division
>>> | NEC Solution Innovators
>>> | tatsu@ab.jp.nec.com
>>>
>>> ------------------------------------------------------------------------
>>> drivers/net/bonding/bond_alb.c | 11 ++++++-----
>>> drivers/net/bonding/bond_main.c | 5 +++--
>>> 2 files changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>>> index c02cc81..7710f20 100644
>>> --- a/drivers/net/bonding/bond_alb.c
>>> +++ b/drivers/net/bonding/bond_alb.c
>>> @@ -1317,7 +1317,7 @@ void bond_alb_deinitialize(struct bonding *bond)
>>> }
>>>
>>> static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>>> - struct slave *tx_slave)
>>> + struct slave *tx_slave, int tlb_dynamic_lb)
>>> {
>>> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>>> struct ethhdr *eth_data = eth_hdr(skb);
>>> @@ -1325,7 +1325,7 @@ static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>>> if (!tx_slave) {
>>> /* unbalanced or unassigned, send through primary */
>>> tx_slave = rcu_dereference(bond->curr_active_slave);
>>> - if (bond->params.tlb_dynamic_lb)
>>> + if (tlb_dynamic_lb)
>>> bond_info->unbalanced_load += skb->len;
>>> }
>>>
>>> @@ -1339,7 +1339,7 @@ static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>>> goto out;
>>> }
>>>
>>> - if (tx_slave && bond->params.tlb_dynamic_lb) {
>>> + if (tx_slave && tlb_dynamic_lb) {
>>> spin_lock(&bond->mode_lock);
>>> __tlb_clear_slave(bond, tx_slave, 0);
>>> spin_unlock(&bond->mode_lock);
>>> @@ -1386,7 +1386,8 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>> break;
>>> }
>>> }
>>> - return bond_do_alb_xmit(skb, bond, tx_slave);
>>> + return bond_do_alb_xmit(skb, bond, tx_slave,
>>> + bond->params.tlb_dynamic_lb);
>>> }
>>>
>>> int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>> @@ -1483,7 +1484,7 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>> tx_slave = tlb_choose_channel(bond, hash_index, skb->len);
>>> }
>>>
>>> - return bond_do_alb_xmit(skb, bond, tx_slave);
>>> + return bond_do_alb_xmit(skb, bond, tx_slave, 1);
>>> }
>>>
>>> void bond_alb_monitor(struct work_struct *work)
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index fc63992..bcb71e7 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -3305,7 +3305,8 @@ static int bond_open(struct net_device *bond_dev)
>>> */
>>> if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
>>> return -ENOMEM;
>>> - if (bond->params.tlb_dynamic_lb)
>>> + if (bond->params.tlb_dynamic_lb ||
>>> + (BOND_MODE(bond) == BOND_MODE_TLB))
>>
>> mode == tlb ? shouldn't this check be for alb ?
>>
> Actually this is not needed since this is already inside if
> (bond_is_lb()) condition.
>
>>> queue_delayed_work(bond->wq, &bond->alb_work, 0);
>>> }
>>>
>>> @@ -4601,7 +4602,7 @@ static int bond_check_params(struct bond_params *params)
>>> }
>>> ad_user_port_key = valptr->value;
>>>
>>> - if ((bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) {
>>> + if (bond_mode == BOND_MODE_TLB) {
>>> bond_opt_initstr(&newval, "default");
>>> valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB),
>>> &newval);
>>>
>>
> I think the underlying issue is that tlb_dynamic_lb should be set to 1
> for all modes which was not the case when it was getting initialized
> only forTLB mode. So from that perspective I prefer Nik's patch with a
> small variation that guards the case when mode transitions from TLB to
> ALB. The reason why I like that patch is because it's simple and
> avoids complications.
+1, I think this is the most straight-forward solution as well and
safest for -net
I will go ahead and submit it in a few minutes.
Thanks,
Nik
>
> Here is what I meant -
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index fc63992ab0e0..c99dc59d729b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4289,7 +4289,7 @@ static int bond_check_params(struct bond_params *params)
> int bond_mode = BOND_MODE_ROUNDROBIN;
> int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
> int lacp_fast = 0;
> - int tlb_dynamic_lb = 0;
> + int tlb_dynamic_lb;
>
> /* Convert string parameters. */
> if (mode) {
> @@ -4601,16 +4601,13 @@ static int bond_check_params(struct bond_params *params)
> }
> ad_user_port_key = valptr->value;
>
> - if ((bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) {
> - bond_opt_initstr(&newval, "default");
> - valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB),
> - &newval);
> - if (!valptr) {
> - pr_err("Error: No tlb_dynamic_lb default value");
> - return -EINVAL;
> - }
> - tlb_dynamic_lb = valptr->value;
> + bond_opt_initstr(&newval, "default");
> + valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB), &newval);
> + if (!valptr) {
> + pr_err("Error: No tlb_dynamic_lb default value");
> + return -EINVAL;
> }
> + tlb_dynamic_lb = valptr->value;
>
> if (lp_interval == 0) {
> pr_warn("Warning: ip_interval must be between 1 and
> %d, so it was reset to %d\n",
> diff --git a/drivers/net/bonding/bond_options.c
> b/drivers/net/bonding/bond_options.c
> index a12d603d41c6..7feacd262182 100644
> --- a/drivers/net/bonding/bond_options.c
> +++ b/drivers/net/bonding/bond_options.c
> @@ -754,6 +754,12 @@ static int bond_option_mode_set(struct bonding *bond,
> bond->params.miimon);
> }
>
> + /* Guard against transition TLB/tlb_dynamic_lb=0 -> ALB mode.
> + * In ALB mode, tlb_dynamic_lb must be set to 1.
> + */
> + if (newval->value == BOND_MODE_ALB && bond->params.tlb_dynamic_lb != 1)
> + bond->params.tlb_dynamic_lb = 1;
> +
> /* don't cache arp_validate between modes */
> bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
> bond->params.mode = newval->value;
>
^ permalink raw reply
* Re: [PATCH] ipv4: Namespaceify tcp_max_orphans knob
From: 严海双 @ 2017-09-09 10:21 UTC (permalink / raw)
To: David Miller
Cc: xiyou.wangcong, kuznet, yoshfuji, edumazet, netdev, linux-kernel
In-Reply-To: <20170908.221648.186026315535806669.davem@davemloft.net>
> On 2017年9月9日, at 下午1:16, David Miller <davem@davemloft.net> wrote:
>
> From: 严海双 <yanhaishuang@cmss.chinamobile.com>
> Date: Sat, 9 Sep 2017 13:09:57 +0800
>
>>
>>
>>> On 2017年9月9日, at 下午12:35, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>
>>> On Fri, Sep 8, 2017 at 6:25 PM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote:
>>>>
>>>>
>>>>> On 2017年9月9日, at 上午6:13, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>>
>>>>> On Wed, Sep 6, 2017 at 8:10 PM, Haishuang Yan
>>>>> <yanhaishuang@cmss.chinamobile.com> wrote:
>>>>>> Different namespace application might require different maximal number
>>>>>> of TCP sockets independently of the host.
>>>>>
>>>>> So after your patch we could have N * net->ipv4.sysctl_tcp_max_orphans
>>>>> in a whole system, right? This just makes OOM easier to trigger.
>>>>>
>>>>
>>>> From my understanding, before the patch, we had N * net->ipv4.sysctl_tcp_max_orphans,
>>>> and after the patch, we could have ns1.sysctl_tcp_max_orphans + ns2.sysctl_tcp_max_orphans
>>>> + ns3.sysctl_tcp_max_orphans, is that right? Thanks for your reviewing.
>>>
>>> Nope, by N I mean the number of containers. Before your patch, the limit
>>> is global, after your patch it is per container.
>>>
>>
>> Yeah, for example, if there is N containers, before the patch, I mean the limit is:
>>
>> N * net->ipv4.sysctl_tcp_max_orphans
>>
>> After the patch, the limit is:
>>
>> ns1. net->ipv4.sysctl_tcp_max_orphans + ns2. net->ipv4.sysctl_tcp_max_orphans + …
>
> Not true.
>
> Please remove "N" from your equation of the current situation.
>
> "sysctl_tcp_max_orphans" applies to entire system, it is a global limit,
> comparing one limit against all orphans in the system, there is no N.
Yes, it’s right. I browse the source code and found that it’s a global limit,
sorry for my mistake.
Thanks David and Cong.
^ permalink raw reply
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Paweł Staszewski @ 2017-09-09 9:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paolo Abeni, Jesper Dangaard Brouer,
Linux Kernel Network Developers, Alexander Duyck
In-Reply-To: <1502759849.4936.46.camel@edumazet-glaptop3.roam.corp.google.com>
Hi
Are there any plans to have this fix normally in kernel ?
Or it is mostly only hack - not longterm fix and need to be different ?
All tests that was done shows that without this patch there is about
20-30% network forwarding performance degradation when using vlan
interfaces
Thanks
Paweł
W dniu 2017-08-15 o 03:17, Eric Dumazet pisze:
> On Mon, 2017-08-14 at 18:07 -0700, Eric Dumazet wrote:
>
>> Or try to hack the IFF_XMIT_DST_RELEASE flag on the vlan netdev.
> Something like :
>
> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index 5e831de3103e2f7092c7fa15534def403bc62fb4..9472de846d5c0960996261cb2843032847fa4bf7 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -143,6 +143,7 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev,
> vlan->vlan_proto = proto;
> vlan->vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
> vlan->real_dev = real_dev;
> + dev->priv_flags |= (real_dev->priv_flags & IFF_XMIT_DST_RELEASE);
> vlan->flags = VLAN_FLAG_REORDER_HDR;
>
> err = vlan_check_real_dev(real_dev, vlan->vlan_proto, vlan->vlan_id);
>
>
>
>
^ permalink raw reply
* [PATCH v2 net] net: qualcomm: rmnet: Fix a double free
From: Dan Carpenter @ 2017-09-09 8:58 UTC (permalink / raw)
To: David S. Miller, Subash Abhinov Kasiviswanathan; +Cc: netdev, kernel-janitors
In-Reply-To: <2ddbb6da514108b4e70ccd8362292134@codeaurora.org>
There is a typo here so we accidentally free "skb" instead of "skbn".
It leads to a double free and a leak. After discussing with Subash,
it's better to just move the check before the allocation and avoid the
need to free.
Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Fix the leak as well. Thanks Subash!
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 557c9bf1a469..86b8c758f94e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -84,6 +84,10 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
if (((int)skb->len - (int)packet_len) < 0)
return NULL;
+ /* Some hardware can send us empty frames. Catch them */
+ if (ntohs(maph->pkt_len) == 0)
+ return NULL;
+
skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
if (!skbn)
return NULL;
@@ -94,11 +98,5 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
memcpy(skbn->data, skb->data, packet_len);
skb_pull(skb, packet_len);
- /* Some hardware can send us empty frames. Catch them */
- if (ntohs(maph->pkt_len) == 0) {
- kfree_skb(skb);
- return NULL;
- }
-
return skbn;
}
^ permalink raw reply related
* Re: [PATCH net-next v8] openvswitch: enable NSH support
From: kbuild test robot @ 2017-09-09 7:10 UTC (permalink / raw)
To: Yi Yang
Cc: kbuild-all, netdev, davem, dev, jbenc, e, blp, jan.scheurich,
Yi Yang
In-Reply-To: <1504850299-84140-1-git-send-email-yi.y.yang@intel.com>
[-- Attachment #1: Type: text/plain, Size: 677 bytes --]
Hi Yi,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Yi-Yang/openvswitch-enable-NSH-support/20170909-124643
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "skb_pop_nsh" [net/openvswitch/openvswitch.ko] undefined!
>> ERROR: "skb_push_nsh" [net/openvswitch/openvswitch.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39620 bytes --]
^ permalink raw reply
* [PATCH v1 3/3] net: skb_queue_purge(): lock/unlock the queue only once
From: Michael Witten @ 2017-09-09 5:50 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: Stephen Hemminger, Eric Dumazet, netdev, linux-kernel
In-Reply-To: <60c8906b751d4915be456009c220516e-mfwitten@gmail.com>
Thanks for your input, Eric Dumazet and Stephen Hemminger; based on
your observations, this version of the patch implements a very
lightweight purging of the queue.
To apply this patch, save this email to:
/path/to/email
and then run:
git am --scissors /path/to/email
You may also fetch this patch from GitHub:
git checkout -b test 5969d1bb3082b41eba8fd2c826559abe38ccb6df
git pull https://github.com/mfwitten/linux.git net/tcp-ip/01-cleanup/02
Sincerely,
Michael Witten
8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----
Hitherto, the queue's lock has been locked/unlocked every time
an item is dequeued; this seems not only inefficient, but also
incorrect, as the whole point of `skb_queue_purge()' is to clear
the queue, presumably without giving any other thread a chance to
manipulate the queue in the interim.
With this commit, the queue's lock is locked/unlocked only once
when `skb_queue_purge()' is called, and in a way that disables
the IRQs for only a minimal amount of time.
This is achieved by atomically re-initializing the queue (thereby
clearing it), and then freeing each of the items as though it were
enqueued in a private queue that doesn't require locking.
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
net/core/skbuff.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 68065d7d383f..bd26b0bde784 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2825,18 +2825,28 @@ struct sk_buff *skb_dequeue_tail(struct sk_buff_head *list)
EXPORT_SYMBOL(skb_dequeue_tail);
/**
- * skb_queue_purge - empty a list
- * @list: list to empty
+ * skb_queue_purge - empty a queue
+ * @q: the queue to empty
*
- * Delete all buffers on an &sk_buff list. Each buffer is removed from
- * the list and one reference dropped. This function takes the list
- * lock and is atomic with respect to other list locking functions.
+ * Dequeue and free each socket buffer that is in @q.
+ *
+ * This function is atomic with respect to other queue-locking functions.
*/
-void skb_queue_purge(struct sk_buff_head *list)
+void skb_queue_purge(struct sk_buff_head *q)
{
- struct sk_buff *skb;
- while ((skb = skb_dequeue(list)) != NULL)
+ unsigned long flags;
+ struct sk_buff *skb, *next, *head = (struct sk_buff *)q;
+
+ spin_lock_irqsave(&q->lock, flags);
+ skb = q->next;
+ __skb_queue_head_init(q);
+ spin_unlock_irqrestore(&q->lock, flags);
+
+ while (skb != head) {
+ next = skb->next;
kfree_skb(skb);
+ skb = next;
+ }
}
EXPORT_SYMBOL(skb_queue_purge);
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next v8] openvswitch: enable NSH support
From: kbuild test robot @ 2017-09-09 5:40 UTC (permalink / raw)
To: Yi Yang
Cc: kbuild-all, netdev, davem, dev, jbenc, e, blp, jan.scheurich,
Yi Yang
In-Reply-To: <1504850299-84140-1-git-send-email-yi.y.yang@intel.com>
[-- Attachment #1: Type: text/plain, Size: 749 bytes --]
Hi Yi,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Yi-Yang/openvswitch-enable-NSH-support/20170909-124643
config: i386-randconfig-a1-201736 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
net/openvswitch/actions.o: In function `do_execute_actions':
>> actions.c:(.text+0x19a7): undefined reference to `skb_push_nsh'
>> actions.c:(.text+0x19ee): undefined reference to `skb_pop_nsh'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27829 bytes --]
^ permalink raw reply
* Re: [PATCH] ipv4: Namespaceify tcp_max_orphans knob
From: David Miller @ 2017-09-09 5:16 UTC (permalink / raw)
To: yanhaishuang
Cc: xiyou.wangcong, kuznet, yoshfuji, edumazet, netdev, linux-kernel
In-Reply-To: <E15E5C56-D712-4FEF-9AC5-AE28857D8D27@cmss.chinamobile.com>
From: 严海双 <yanhaishuang@cmss.chinamobile.com>
Date: Sat, 9 Sep 2017 13:09:57 +0800
>
>
>> On 2017年9月9日, at 下午12:35, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> On Fri, Sep 8, 2017 at 6:25 PM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote:
>>>
>>>
>>>> On 2017年9月9日, at 上午6:13, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>
>>>> On Wed, Sep 6, 2017 at 8:10 PM, Haishuang Yan
>>>> <yanhaishuang@cmss.chinamobile.com> wrote:
>>>>> Different namespace application might require different maximal number
>>>>> of TCP sockets independently of the host.
>>>>
>>>> So after your patch we could have N * net->ipv4.sysctl_tcp_max_orphans
>>>> in a whole system, right? This just makes OOM easier to trigger.
>>>>
>>>
>>> From my understanding, before the patch, we had N * net->ipv4.sysctl_tcp_max_orphans,
>>> and after the patch, we could have ns1.sysctl_tcp_max_orphans + ns2.sysctl_tcp_max_orphans
>>> + ns3.sysctl_tcp_max_orphans, is that right? Thanks for your reviewing.
>>
>> Nope, by N I mean the number of containers. Before your patch, the limit
>> is global, after your patch it is per container.
>>
>
> Yeah, for example, if there is N containers, before the patch, I mean the limit is:
>
> N * net->ipv4.sysctl_tcp_max_orphans
>
> After the patch, the limit is:
>
> ns1. net->ipv4.sysctl_tcp_max_orphans + ns2. net->ipv4.sysctl_tcp_max_orphans + …
Not true.
Please remove "N" from your equation of the current situation.
"sysctl_tcp_max_orphans" applies to entire system, it is a global limit,
comparing one limit against all orphans in the system, there is no N.
^ permalink raw reply
* Re: [PATCH] ipv4: Namespaceify tcp_max_orphans knob
From: 严海双 @ 2017-09-09 5:09 UTC (permalink / raw)
To: Cong Wang
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Eric Dumazet, Linux Kernel Network Developers, LKML
In-Reply-To: <CAM_iQpVDM0HwZd6c+Loych=nQE__eGteeJQ=XTOfHKgVxMRfrA@mail.gmail.com>
> On 2017年9月9日, at 下午12:35, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Fri, Sep 8, 2017 at 6:25 PM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote:
>>
>>
>>> On 2017年9月9日, at 上午6:13, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>
>>> On Wed, Sep 6, 2017 at 8:10 PM, Haishuang Yan
>>> <yanhaishuang@cmss.chinamobile.com> wrote:
>>>> Different namespace application might require different maximal number
>>>> of TCP sockets independently of the host.
>>>
>>> So after your patch we could have N * net->ipv4.sysctl_tcp_max_orphans
>>> in a whole system, right? This just makes OOM easier to trigger.
>>>
>>
>> From my understanding, before the patch, we had N * net->ipv4.sysctl_tcp_max_orphans,
>> and after the patch, we could have ns1.sysctl_tcp_max_orphans + ns2.sysctl_tcp_max_orphans
>> + ns3.sysctl_tcp_max_orphans, is that right? Thanks for your reviewing.
>
> Nope, by N I mean the number of containers. Before your patch, the limit
> is global, after your patch it is per container.
>
Yeah, for example, if there is N containers, before the patch, I mean the limit is:
N * net->ipv4.sysctl_tcp_max_orphans
After the patch, the limit is:
ns1. net->ipv4.sysctl_tcp_max_orphans + ns2. net->ipv4.sysctl_tcp_max_orphans + …
^ permalink raw reply
* Re: [PATCH net-next 1/1] sched: Use __qdisc_drop instead of kfree_skb in sch_prio and sch_qfq
From: David Miller @ 2017-09-07 4:20 UTC (permalink / raw)
To: gfree.wind; +Cc: jhs, xiyou.wangcong, jiri, edumazet, netdev
In-Reply-To: <1504506072-46207-1-git-send-email-gfree.wind@vip.163.com>
From: gfree.wind@vip.163.com
Date: Mon, 4 Sep 2017 14:21:12 +0800
> From: Gao Feng <gfree.wind@vip.163.com>
>
> The commit 520ac30f4551 ("net_sched: drop packets after root qdisc lock
> is released) made a big change of tc for performance. There are two points
> left in sch_prio and sch_qfq which are not changed with that commit. Now
> enhance them now with __qdisc_drop.
>
> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
Thanks for catching these missed cases, applied.
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2017-09-09 5:06 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
The iwlwifi firmware compat fix is in here as well as some other
stuff:
1) Fix request socket leak introduced by BPF deadlock fix, from Eric
Dumazet.
2) Fix VLAN handling with TXQs in mac80211, from Johannes Berg.
3) Missing __qdisc_drop conversions in prio and qfq schedulers, from
Gao Feng.
4) Use after free in netlink nlk groups handling, from Xin Long.
5) Handle MTU update properly in ipv6 gre tunnels, from Xin Long.
6) Fix leak of ipv6 fib tables on netns teardown, from Sabrina Dubroca
with follow-on fix from Eric Dumazet.
7) Need RCU and preemption disabled during generic XDP data patch, from
John Fastabend.
Please pull, thanks a lot!
The following changes since commit 80cee03bf1d626db0278271b505d7f5febb37bba:
Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 (2017-09-06 15:17:17 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
for you to fetch changes up to 9beb8bedb05c5f3a353dba62b8fa7cbbb9ec685e:
bpf: make error reporting in bpf_warn_invalid_xdp_action more clear (2017-09-08 21:13:09 -0700)
----------------------------------------------------------------
Arnd Bergmann (1):
isdn: isdnloop: fix logic error in isdnloop_sendbuf
Avraham Stern (1):
mac80211: flush hw_roc_start work before cancelling the ROC
Baruch Siach (5):
dt-binding: phy: don't confuse with Ethernet phy properties
dt-bindings: net: don't confuse with generic PHY property
dt-bindings: add SFF vendor prefix
dt-binding: net: sfp binding documentation
net: phy: sfp: rename dt properties to match the binding
Beni Lev (1):
mac80211_hwsim: Use proper TX power
Chunho Lee (1):
mac80211: Fix null pointer dereference with iTXQ support
Daniel Borkmann (2):
bpf: don't select potentially stale ri->map from buggy xdp progs
bpf: make error reporting in bpf_warn_invalid_xdp_action more clear
David S. Miller (4):
Merge tag 'mac80211-for-davem-2017-09-07' of git://git.kernel.org/.../jberg/mac80211
Merge tag 'wireless-drivers-for-davem-2017-09-08' of git://git.kernel.org/.../kvalo/wireless-drivers
Merge git://git.kernel.org/.../pablo/nf
Merge branch 'xdp-bpf-fixes'
Emmanuel Grumbach (1):
cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}
Eric Dumazet (2):
tcp: fix a request socket leak
ipv6: fix typo in fib6_net_exit()
Florian Fainelli (1):
Revert "mdio_bus: Remove unneeded gpiod NULL check"
Florian Westphal (5):
netfilter: nf_nat: don't bug when mapping already exists
netfilter: xtables: add scheduling opportunity in get_counters
netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable"
netfilter: nat: use keyed locks
netfilter: core: remove erroneous warn_on
Gao Feng (1):
sched: Use __qdisc_drop instead of kfree_skb in sch_prio and sch_qfq
Haishuang Yan (2):
ip_tunnel: fix setting ttl and tos value in collect_md mode
ip6_tunnel: fix setting hop_limit value for ipv6 tunnel
Håkon Bugge (1):
rds: Fix incorrect statistics counting
Ian W MORRISON (1):
brcmfmac: feature check for multi-scheduled scan fails on bcm4345 devices
Igor Mitsyanko (1):
nl80211: look for HT/VHT capabilities in beacon's tail
Ilan peer (1):
mac80211: Complete ampdu work schedule during session tear down
Ivan Khoronzhuk (1):
net: ethernet: ti: netcp_core: no need in netif_napi_del
Jiri Pirko (1):
net: sched: fix memleak for chain zero
Johannes Berg (3):
mac80211: fix VLAN handling with TXQs
mac80211: agg-tx: call drv_wake_tx_queue in proper context
mac80211: fix deadlock in driver-managed RX BA session start
John Fastabend (3):
net: rcu lock and preempt disable missing around generic xdp
bpf: add support for sockmap detach programs
bpf: devmap, use cond_resched instead of cpu_relax
Kees Cook (1):
net: tulip: Constify tulip_tbl
Kleber Sacilotto de Souza (1):
tipc: remove unnecessary call to dev_net()
Larry Finger (2):
rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be
rtlwifi: btcoexist: Fix antenna selection code
Liad Kaufman (1):
mac80211: add MESH IE in the correct order
Luca Coelho (1):
iwlwifi: mvm: only send LEDS_CMD when the FW supports it
Marcelo Ricardo Leitner (1):
sctp: fix missing wake ups in some situations
Mathieu Malaterre (1):
davicom: Display proper debug level up to 6
Paolo Abeni (1):
udp: drop head states only when all skb references are gone
Sabrina Dubroca (1):
ipv6: fix memory leak with multiple tables during netns destruction
Sharon Dvir (1):
mac80211: shorten debug prints using ht_dbg() to avoid warning
Simon Dinkin (1):
mac80211: fix incorrect assignment of reassoc value
Vishwanath Pai (1):
netfilter: xt_hashlimit: fix build error caused by 64bit division
Xin Long (5):
netlink: fix an use-after-free issue for nlk groups
netlink: access nlk groups safely in netlink bind and getname
ip6_gre: update mtu properly in ip6gre_err
netfilter: ipvs: fix the issue that sctp_conn_schedule drops non-INIT packet
netfilter: ipvs: do not create conn for ABORT packet in sctp_conn_schedule
Zhizhou Tian (1):
netfilter: xt_hashlimit: alloc hashtable with right size
Documentation/devicetree/bindings/net/ethernet.txt | 4 +++
Documentation/devicetree/bindings/net/sff,sfp.txt | 76 ++++++++++++++++++++++++++++++++++++++++++++++
Documentation/devicetree/bindings/phy/phy-bindings.txt | 4 ++-
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/isdn/isdnloop/isdnloop.c | 2 +-
drivers/net/ethernet/davicom/dm9000.c | 2 +-
drivers/net/ethernet/dec/tulip/tulip.h | 2 +-
drivers/net/ethernet/dec/tulip/tulip_core.c | 2 +-
drivers/net/ethernet/ti/netcp_core.c | 1 -
drivers/net/phy/mdio_bus.c | 6 ++--
drivers/net/phy/sfp.c | 4 +--
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 3 +-
drivers/net/wireless/intel/iwlwifi/fw/file.h | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/led.c | 3 +-
drivers/net/wireless/mac80211_hwsim.c | 2 --
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c | 5 +++-
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 23 +++++++++-----
include/linux/bpf.h | 8 ++---
include/linux/skbuff.h | 2 +-
include/net/mac80211.h | 15 ++--------
include/net/netfilter/nf_conntrack.h | 3 +-
include/net/netfilter/nf_nat.h | 1 -
include/uapi/linux/bpf.h | 4 +--
kernel/bpf/devmap.c | 2 +-
kernel/bpf/sockmap.c | 2 +-
kernel/bpf/syscall.c | 27 ++++++++++-------
kernel/bpf/verifier.c | 16 ++++++++++
net/core/dev.c | 25 ++++++++++------
net/core/filter.c | 27 +++++++++++++++--
net/core/skbuff.c | 9 ++----
net/ipv4/ip_tunnel.c | 4 +--
net/ipv4/netfilter/arp_tables.c | 1 +
net/ipv4/netfilter/ip_tables.c | 1 +
net/ipv4/tcp_ipv4.c | 6 ++--
net/ipv4/udp.c | 5 +++-
net/ipv6/ip6_fib.c | 25 ++++++++++++----
net/ipv6/ip6_gre.c | 4 ++-
net/ipv6/ip6_tunnel.c | 1 +
net/ipv6/netfilter/ip6_tables.c | 1 +
net/ipv6/tcp_ipv6.c | 6 ++--
net/mac80211/agg-rx.c | 32 +++++++++++++-------
net/mac80211/agg-tx.c | 8 +++--
net/mac80211/ht.c | 24 +++++++++++++--
net/mac80211/ieee80211_i.h | 4 +++
net/mac80211/iface.c | 20 +++++++++++--
net/mac80211/mlme.c | 2 +-
net/mac80211/offchannel.c | 2 ++
net/mac80211/tx.c | 36 +++++++++++++++++-----
net/mac80211/util.c | 2 +-
net/netfilter/core.c | 2 +-
net/netfilter/ipvs/ip_vs_proto_sctp.c | 8 +++--
net/netfilter/nf_nat_core.c | 146 +++++++++++++++++++++++++++++++++++++++++------------------------------------------------
net/netfilter/xt_hashlimit.c | 16 +++++-----
net/netlink/af_netlink.c | 22 +++++++++-----
net/rds/send.c | 10 +++++--
net/sched/cls_api.c | 18 +++++------
net/sched/sch_prio.c | 2 +-
net/sched/sch_qfq.c | 2 +-
net/sctp/ulpqueue.c | 3 +-
net/tipc/bearer.c | 2 +-
net/wireless/nl80211.c | 4 +--
net/wireless/reg.c | 20 +++++++++++--
tools/testing/selftests/bpf/test_maps.c | 51 ++++++++++++++++++++++++++++++-
63 files changed, 537 insertions(+), 235 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/sff,sfp.txt
^ 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