* Re: [patch net-next RFC 4/6] Introduce sample tc action
From: Roopa Prabhu @ 2016-10-18 5:07 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, netdev, davem, yotamg, idosch, eladr, nogahf,
ogerlitz, geert+renesas, stephen, xiyou.wangcong, linux,
Shrijeet Mukherjee, Peter Phaal
In-Reply-To: <58056A08.5070809@cumulusnetworks.com>
On 10/17/16, 5:17 PM, Roopa Prabhu wrote:
> On 10/17/16, 3:10 AM, Jamal Hadi Salim wrote:
[snip]
inline below more data/context..
>>>> +
>>>> +struct sample_packet_metadata {
>>>> + int sample_size;
>>>> + int orig_size;
>>>> + int ifindex;
>>>> +};
>>>> +
>>> This metadata does not look extensible.. can it be made to ?
>>>
>> Sure it can...
more sflow context here... [1]
An extensible metadata scheme is highly desirable when passing data from the dataplane to
the sampling agent in userspace. Looking forward, advanced instrumentation is being
added to data planes and keeping the api future proof will help.
>>
>>> With sflow in context, you need a pair of ifindex numbers to encode ingress and egress ports.
>> What is the use case for both?
> I have heard that most monitoring tools have moved to ingress only sampling because of operational
> complexity (use case is sflow). I think hardware also supports ingress and egress only sampling.
> better to have an option to reflect that in the api.
The reason for having two ifindex numbers is to record the ingress and egress ports (i.e. the path that the packet takes through the datapath/ASIC). You may actually have three ifindex numbers associated with a sample:
1. The data source that made the measurement (on a linux system each bridge has its own ifindex)
2. The ifindex associated with the ingress switch port
3. The ifindex associated with the egress switch port.
All three apply irrespective of sampling direction.
thanks,
Roopa
[1] Additional extended flow attributes have been defined to further extend sFlow packet samples:
http://sflow.org/sflow_tunnels.txt <http://sflow.org/sflow_tunnels.txt>
http://sflow.org/sflow_openflow.txt <http://sflow.org/sflow_openflow.txt>
^ permalink raw reply
* Useless debug warning "netlink: 16 bytes leftover after parsing attributes"
From: Marcel Holtmann @ 2016-10-18 5:06 UTC (permalink / raw)
To: Network Development; +Cc: Michal Schmidt
Hi,
so lately I am seeing a bunch of these warnings:
netlink: 16 bytes leftover after parsing attributes..
While they give you the process name, they are still useless to track down the message that causes them. I find them even more useless since an updated userspace on an older kernel can trigger the nla_policy warning here. And that updated userspace program is doing nothing wrong by including extra attributes. So what purpose is this warning serving?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Tom Herbert @ 2016-10-18 4:28 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: Yuchung Cheng, netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <D42AE34E.1FF9D%brakmo@fb.com>
On Mon, Oct 17, 2016 at 8:35 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
> Yuchung and Eric, thank you for your comments.
>
> It looks like I need to think more about this patch. I was trying
> to reduce the likelihood of reordering (which seems even more
> important based on Eric¹s comment on pacing), but it seems like
> the only way to prevent reordering is to only re-hash after an RTO
> or when there are no packets in flight (which may not occur).
>
Sounds like that should be the same condition as when we set ooo_okay?
>
> On 10/11/16, 8:56 PM, "Yuchung Cheng" <ycheng@google.com> wrote:
>
>>On Tue, Oct 11, 2016 at 6:01 PM, Yuchung Cheng <ycheng@google.com> wrote:
>>> On Tue, Oct 11, 2016 at 2:08 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>>>> Yuchung, thank you for your comments. Responses inline.
>>>>
>>>> On 10/11/16, 12:49 PM, "Yuchung Cheng" <ycheng@google.com> wrote:
>>>>
>>>>>On Mon, Oct 10, 2016 at 5:18 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>>>>>>
>>>>>> The purpose of this patch is to help balance flows across paths. A
>>>>>>new
>>>>>> sysctl "tcp_retrans_txhash_prob" specifies the probability (0-100)
>>>>>>that
>>>>>> the txhash (IPv6 flowlabel) will be changed after a non-RTO
>>>>>>retransmit.
>>>>>> A probability is used in order to control how many flows are moved
>>>>>> during a congestion event and prevent the congested path from
>>>>>>becoming
>>>>>> under utilized (which could occur if too many flows leave the current
>>>>>> path). Txhash changes may be delayed in order to decrease the
>>>>>>likelihood
>>>>>> that it will trigger retransmists due to too much reordering.
>>>>>>
>>>>>> Another sysctl "tcp_retrans_txhash_mode" determines the behavior
>>>>>>after
>>>>>> RTOs. If the sysctl is 0, then after an RTO, only RTOs can trigger
>>>>>> txhash changes. The idea is to decrease the likelihood of going back
>>>>>> to a broken path. That is, we don't want flow balancing to trigger
>>>>>> changes to broken paths. The drawback is that flow balancing does
>>>>>> not work as well. If the sysctl is greater than 1, then we always
>>>>>> do flow balancing, even after RTOs.
>>>>>>
>>>>>> Tested with packedrill tests (for correctness) and performance
>>>>>> experiments with 2 and 3 paths. Performance experiments looked at
>>>>>> aggregate goodput and fairness. For each run, we looked at the ratio
>>>>>>of
>>>>>> the goodputs for the fastest and slowest flows. These were averaged
>>>>>>for
>>>>>> all the runs. A fairness of 1 means all flows had the same goodput, a
>>>>>> fairness of 2 means the fastest flow was twice as fast as the slowest
>>>>>> flow.
>>>>>>
>>>>>> The setup for the performance experiments was 4 or 5 serves in a
>>>>>>rack,
>>>>>> 10G links. I tested various probabilities, but 20 seemed to have the
>>>>>> best tradeoff for my setup (small RTTs).
>>>>>>
>>>>>> --- node1 -----
>>>>>> sender --- switch --- node2 ----- switch ---- receiver
>>>>>> --- node3 -----
>>>>>>
>>>>>> Scenario 1: One sender sends to one receiver through 2 routes (node1
>>>>>>or
>>>>>> node 2). The output from node1 and node2 is 1G (1gbit/sec). With
>>>>>>only 2
>>>>>> flows, without flow balancing (prob=0) the average goodput is 1.6G
>>>>>>vs.
>>>>>> 1.9G with flow balancing due to 2 flows ending up in one link and
>>>>>>either
>>>>>> not moving and taking some time to move. Fairness was 1 in all cases.
>>>>>> For 7 flows, goodput was 1.9G for all, but fairness was 1.5, 1.4 or
>>>>>>1.2
>>>>>> for prob=0, prob=20,mode=0 and prob=20,mode=1 respectively. That is,
>>>>>> flow balancing increased fairness.
>>>>>>
>>>>>> Scenario 2: One sender to one receiver, through 3 routes (node1,...
>>>>>> node2). With 6 or 16 flows the goodput was the same for all, but
>>>>>> fairness was 1.8, 1.5 and 1.2 respectively. Interestingly, the worst
>>>>>> case fairness out of 10 runs were 2.2, 1.8 and 1.4 repectively. That
>>>>>>is,
>>>>>> prob=20,mode=1 improved average and worst case fairness.
>>>>>I am wondering if we can build better API with routing layer to
>>>>>implement this type of feature, instead of creeping the tx_rehashing
>>>>>logic scatter in TCP. For example, we call dst_negative_advice on TCP
>>>>>write timeouts.
>>>>
>>>> Not sure. The route is not necessarily bad, may be temporarily
>>>>congested
>>>> or they may all be congested. If all we want to do is change the txhash
>>>> (unlike dst_negative_advice), then calling a tx_rehashing function may
>>>> be the appropriate call.
>>>>
>>>>>
>>>>>On the patch itself, it seems aggressive to (attempt to) rehash every
>>>>>post-RTO retranmission. Also you can just use ca_state (==CA_Loss) to
>>>>>identify post-RTO retransmission directly.
>>>>
>>>> Thanks, I will add the test.
>>>>
>>>>>
>>>>>is this an implementation of the Flow Bender ?
>>>>>https://urldefense.proofpoint.com/v2/url?u=http-3A__dl.acm.org_citation
>>>>>.cf
>>>>>m-3Fid-3D2674985&d=DQIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8ltkgyx
>>>>>1u_
>>>>>g&m=Q4nONH7kQ5AvQguw9UxpcHd79jfdDdrXj1YSJs7Ezhk&s=MA4fWBLMTGgRS0eGvBjxf
>>>>>7BJ
>>>>>Ol3-oxAzZDEYUG4cE-s&e=
>>>>
>>>> Part of flow bender, although there are also some similarities to
>>>>flowlet
>>>> switching.
>>>>
>>>>>
>>>>>>
>>>>>> Scenario 3: One sender to one receiver, 2 routes, one route drops
>>>>>>50% of
>>>>>> the packets. With 7 flows, goodput was the same 1.1G, but fairness
>>>>>>was
>>>>>> 1.8, 2.0 and 2.1 respectively. That is, if there is a bad route, then
>>>>>> balancing, which does more re-routes, is less fair.
>>>>>>
>>>>>> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
>>>>>> ---
>>>>>> Documentation/networking/ip-sysctl.txt | 15 +++++++++++++++
>>>>>> include/linux/tcp.h | 4 +++-
>>>>>> include/net/tcp.h | 2 ++
>>>>>> net/ipv4/sysctl_net_ipv4.c | 18 ++++++++++++++++++
>>>>>> net/ipv4/tcp_input.c | 10 ++++++++++
>>>>>> net/ipv4/tcp_output.c | 23 ++++++++++++++++++++++-
>>>>>> net/ipv4/tcp_timer.c | 4 ++++
>>>>>> 7 files changed, 74 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/networking/ip-sysctl.txt
>>>>>>b/Documentation/networking/ip-sysctl.txt
>>>>>> index 3db8c67..87a984c 100644
>>>>>> --- a/Documentation/networking/ip-sysctl.txt
>>>>>> +++ b/Documentation/networking/ip-sysctl.txt
>>>>>> @@ -472,6 +472,21 @@ tcp_max_reordering - INTEGER
>>>>>> if paths are using per packet load balancing (like bonding rr
>>>>>>mode)
>>>>>> Default: 300
>>>>>>
>>>>>> +tcp_retrans_txhash_mode - INTEGER
>>>>>> + If zero, disable txhash recalculation due to non-RTO
>>>>>>retransmissions
>>>>>> + after an RTO. The idea is that broken paths will trigger an
>>>>>>RTO
>>>>>>and
>>>>>> + we don't want going back to that path due to standard
>>>>>>retransmissons
>>>>>> + (flow balancing). The drawback is that balancing is less
>>>>>>robust.
>>>>>> + If greater than zero, can always (probabilistically)
>>>>>>recalculate
>>>>>> + txhash after non-RTO retransmissions.
>>>>>> +
>>>>>> +tcp_retrans_txhash_prob - INTEGER
>>>>>> + Probability [0 to 100] that we will recalculate txhash when a
>>>>>> + packet is resent not due to RTO (for RTO txhash is always
>>>>>>recalculated).
>>>>>> + The recalculation of the txhash may be delayed to decrease
>>>>>>the
>>>>>> + likelihood that reordering will trigger retransmissons.
>>>>>> + The purpose is to help balance the flows among the possible
>>>>>>paths.
>>>>>> +
>>>>>> tcp_retrans_collapse - BOOLEAN
>>>>>> Bug-to-bug compatibility with some broken printers.
>>>>>> On retransmit try to send bigger packets to work around bugs
>>>>>>in
>>>>>> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
>>>>>> index a17ae7b..e0e3b7d 100644
>>>>>> --- a/include/linux/tcp.h
>>>>>> +++ b/include/linux/tcp.h
>>>>>> @@ -214,7 +214,9 @@ struct tcp_sock {
>>>>>> } rack;
>>>>>> u16 advmss; /* Advertised MSS
>>>>>>*/
>>>>>> u8 rate_app_limited:1, /* rate_{delivered,interval_us}
>>>>>>limited? */
>>>>>> - unused:7;
>>>>>> + txhash_rto:1, /* If set, don't do flow balancing
>>>>>>*/
>>>>>> + txhash_want:1, /* We want to change txhash when safe
>>>>>>*/
>>>>>> + unused:5;
>>>>>> u8 nonagle : 4,/* Disable Nagle algorithm?
>>>>>>*/
>>>>>> thin_lto : 1,/* Use linear timeouts for thin
>>>>>>streams
>>>>>>*/
>>>>>> thin_dupack : 1,/* Fast retransmit on first dupack
>>>>>>*/
>>>>>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>>>>>> index f83b7f2..3abd304 100644
>>>>>> --- a/include/net/tcp.h
>>>>>> +++ b/include/net/tcp.h
>>>>>> @@ -271,6 +271,8 @@ extern int sysctl_tcp_autocorking;
>>>>>> extern int sysctl_tcp_invalid_ratelimit;
>>>>>> extern int sysctl_tcp_pacing_ss_ratio;
>>>>>> extern int sysctl_tcp_pacing_ca_ratio;
>>>>>> +extern int sysctl_tcp_retrans_txhash_prob;
>>>>>> +extern int sysctl_tcp_retrans_txhash_mode;
>>>>>>
>>>>>> extern atomic_long_t tcp_memory_allocated;
>>>>>> extern struct percpu_counter tcp_sockets_allocated;
>>>>>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
>>>>>> index 1cb67de..00d6f26 100644
>>>>>> --- a/net/ipv4/sysctl_net_ipv4.c
>>>>>> +++ b/net/ipv4/sysctl_net_ipv4.c
>>>>>> @@ -28,6 +28,7 @@
>>>>>> static int zero;
>>>>>> static int one = 1;
>>>>>> static int four = 4;
>>>>>> +static int hundred = 100;
>>>>>> static int thousand = 1000;
>>>>>> static int gso_max_segs = GSO_MAX_SEGS;
>>>>>> static int tcp_retr1_max = 255;
>>>>>> @@ -624,6 +625,23 @@ static struct ctl_table ipv4_table[] = {
>>>>>> .proc_handler = proc_dointvec_ms_jiffies,
>>>>>> },
>>>>>> {
>>>>>> + .procname = "tcp_retrans_txhash_prob",
>>>>>> + .data = &sysctl_tcp_retrans_txhash_prob,
>>>>>> + .maxlen = sizeof(int),
>>>>>> + .mode = 0644,
>>>>>> + .proc_handler = proc_dointvec_minmax,
>>>>>> + .extra1 = &zero,
>>>>>> + .extra2 = &hundred,
>>>>>> + },
>>>>>> + {
>>>>>> + .procname = "tcp_retrans_txhash_mode",
>>>>>> + .data = &sysctl_tcp_retrans_txhash_mode,
>>>>>> + .maxlen = sizeof(int),
>>>>>> + .mode = 0644,
>>>>>> + .proc_handler = proc_dointvec_minmax,
>>>>>> + .extra1 = &zero,
>>>>>> + },
>>>>>> + {
>>>>>> .procname = "icmp_msgs_per_sec",
>>>>>> .data = &sysctl_icmp_msgs_per_sec,
>>>>>> .maxlen = sizeof(int),
>>>>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>>>>> index a27b9c0..fed5366 100644
>>>>>> --- a/net/ipv4/tcp_input.c
>>>>>> +++ b/net/ipv4/tcp_input.c
>>>>>> @@ -101,6 +101,9 @@ int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
>>>>>> int sysctl_tcp_early_retrans __read_mostly = 3;
>>>>>> int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
>>>>>>
>>>>>> +int sysctl_tcp_retrans_txhash_prob __read_mostly;
>>>>>> +int sysctl_tcp_retrans_txhash_mode __read_mostly;
>>>>>> +
>>>>>> #define FLAG_DATA 0x01 /* Incoming frame contained
>>>>>>data.
>>>>>> */
>>>>>> #define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a
>>>>>>window update. */
>>>>>> #define FLAG_DATA_ACKED 0x04 /* This ACK acknowledged
>>>>>>new data. */
>>>>>> @@ -3674,6 +3677,13 @@ static int tcp_ack(struct sock *sk, const
>>>>>>struct
>>>>>>sk_buff *skb, int flag)
>>>>>> flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una,
>>>>>>&acked,
>>>>>> &sack_state, &now);
>>>>>>
>>>>>> + /* Check if we should set txhash (would not cause
>>>>>>reordering) */
>>>>>> + if (tp->txhash_want &&
>>>>>> + (tp->packets_out - tp->sacked_out) < tp->reordering) {
>>>>>> + sk_set_txhash(sk);
>>>>>> + tp->txhash_want = 0;
>>>>>> + }
>>>>>> +
>>>>>> if (tcp_ack_is_dubious(sk, flag)) {
>>>>>> is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED |
>>>>>>FLAG_NOT_DUP));
>>>>>> tcp_fastretrans_alert(sk, acked, is_dupack, &flag,
>>>>>>&rexmit);
>>>>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>>>>> index 896e9df..58490ac 100644
>>>>>> --- a/net/ipv4/tcp_output.c
>>>>>> +++ b/net/ipv4/tcp_output.c
>>>>>> @@ -2738,9 +2738,30 @@ int tcp_retransmit_skb(struct sock *sk, struct
>>>>>>sk_buff *skb, int segs)
>>>>>> tp->retrans_out += tcp_skb_pcount(skb);
>>>>>>
>>>>>> /* Save stamp of the first retransmit. */
>>>>>> - if (!tp->retrans_stamp)
>>>>>> + if (!tp->retrans_stamp) {
>>>>>> tp->retrans_stamp = tcp_skb_timestamp(skb);
>>>>>>
>>>>>> + /* Determine if we should reset hash, only
>>>>>>done
>>>>>>once
>>>>>> + * per recovery
>>>>>> + */
>>>>>> + if ((!tp->txhash_rto ||
>>>>>> + sysctl_tcp_retrans_txhash_mode > 0) &&
>>>>>> + sk->sk_txhash &&
>>>>>> + (prandom_u32_max(100) <
>>>>>> + sysctl_tcp_retrans_txhash_prob)) {
>>>>>> + /* If not too much reordering, or
>>>>>>RTT is
>>>>>> + * small enough that we don't care
>>>>>>about
>>>>>> + * reordering, then change it now.
>>>>>> + * Else, wait until it is safe.
>>>>>> + */
>>>>>> + if ((tp->packets_out -
>>>>>>tp->sacked_out) <
>>>>>> + tp->reordering)
>>>>>I don't parse this logic ... suppose reordering is 100 (not uncommon
>>>>>today due to the last packet being delivered slightly earlier than the
>>>>>rest), and cwnd==packets_out =~200,we only want to rehash until half
>>>>>of the packets are sacked, so we are still rehashing even when
>>>>>reordering is heavy?
>>>>
>>>> In your scenario, there would be no re-hashing until sacked_out is 101
>>>> ((packets_out - sacked_out) < 100). This code would mark txhash_want.
>>>> Then when an ACK is received and the conditional is true, txhash
>>>> would be changed.
>>>>
>>>> Now, the test does not prevent retransmissions due to reordering in all
>>>> cases, but hopefully in most. I will also add the test you recommended,
>>>> checking for CA_Loss, to prevent too much re-hashing.
>>> If the whole point is to rehash at most once between recovery events,
>>> why do we need this complicated change at per packet level in
>>> tcp_retransmit_skb()? there are functions that start and end recovery
>>> (tcp_enter_recovery, tcp_end_cwnd_reduction). our retransmission logic
>>> is already very complicated.
>>>
>>> I still don't understand the incentive of starting the rehashing
>>> half-way retransmitting depending on the sacking and reordering
>>> status, for temporarily congestion as you've mentioned.
>>>
>>> is this feature unique for intra-DC connections?
>>I thought more about this patch on my way home and have more
>>questions: why do we exclude RTO retransmission specifically? also
>>when we rehash, we'll introduce reordering either in recovery or after
>>recovery, as some TCP CC like bbr would continue sending regardlessly,
>>so starting in tcp_ack() with tp->txhash_want does not really prevent
>>causing more reordering.
>>
>>so if we want to rehash upon a recovery event (fast or timeout), then
>>we can make a helper function ie tcp_retry_alt_path, and just call it
>>in tcp_enter_loss and tcp_recovery (which are called once per
>>recovery). Since it'll cause reordering anyways, I doubt we need to
>>check tp->reordering at all. If packets already traverse different
>>paths, rehashing can make it better or worse. if packets weren't
>>traversing different paths, the rehashing itself can introduce
>>reordering. so in the end there is much benefit to make decision based
>>on current inflight and reordering IMO...
>>
>>
>>>
>>>>
>>>>>
>>>>>also where do we check RTT is small?
>>>>
>>>> The RTT comment is left over from a previous version, I will remove it.
>>>>
>>>>>
>>>>>> + sk_set_txhash(sk);
>>>>>> + else
>>>>>> + tp->txhash_want = 1;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> } else if (err != -EBUSY) {
>>>>>> NET_INC_STATS(sock_net(sk),
>>>>>>LINUX_MIB_TCPRETRANSFAIL);
>>>>>> }
>>>>>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>>>>>> index 3ea1cf8..e66baad 100644
>>>>>> --- a/net/ipv4/tcp_timer.c
>>>>>> +++ b/net/ipv4/tcp_timer.c
>>>>>> @@ -186,6 +186,8 @@ static int tcp_write_timeout(struct sock *sk)
>>>>>>
>>>>>> if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
>>>>>> if (icsk->icsk_retransmits) {
>>>>>> + tp->txhash_rto = 1;
>>>>>> + tp->txhash_want = 0;
>>>>>> dst_negative_advice(sk);
>>>>>> if (tp->syn_fastopen || tp->syn_data)
>>>>>> tcp_fastopen_cache_set(sk, 0, NULL,
>>>>>>true, 0);
>>>>>> @@ -218,6 +220,8 @@ static int tcp_write_timeout(struct sock *sk)
>>>>>> } else {
>>>>>> sk_rethink_txhash(sk);
>>>>>> }
>>>>>> + tp->txhash_rto = 1;
>>>>>> + tp->txhash_want = 0;
>>>>>>
>>>>>> retry_until = net->ipv4.sysctl_tcp_retries2;
>>>>>> if (sock_flag(sk, SOCK_DEAD)) {
>>>>>> --
>>>>>> 2.9.3
>>>>>>
>>>>
>
^ permalink raw reply
* [PATCH net-next] r8152: add new products of Lenovo
From: Hayes Wang @ 2016-10-18 3:41 UTC (permalink / raw)
To: netdev, oliver; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
Add the following four products of Lenovo and sort the order of the list.
VID PID
0x17ef 0x3062
0x17ef 0x3069
0x17ef 0x720c
0x17ef 0x7214
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/cdc_ether.c | 28 ++++++++++++++++++++++++++++
drivers/net/usb/r8152.c | 6 +++++-
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index c47ec0a..45e5e43 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -687,6 +687,20 @@ static const struct usb_device_id products[] = {
.driver_info = 0,
},
+/* ThinkPad USB-C Dock (based on Realtek RTL8153) */
+{
+ USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3062, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+ .driver_info = 0,
+},
+
+/* ThinkPad Thunderbolt 3 Dock (based on Realtek RTL8153) */
+{
+ USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3069, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+ .driver_info = 0,
+},
+
/* Lenovo Thinkpad USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
{
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7205, USB_CLASS_COMM,
@@ -694,6 +708,20 @@ static const struct usb_device_id products[] = {
.driver_info = 0,
},
+/* Lenovo USB C to Ethernet Adapter (based on Realtek RTL8153) */
+{
+ USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x720c, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+ .driver_info = 0,
+},
+
+/* Lenovo USB-C Travel Hub (based on Realtek RTL8153) */
+{
+ USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7214, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+ .driver_info = 0,
+},
+
/* NVIDIA Tegra USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
{
USB_DEVICE_AND_INTERFACE_INFO(NVIDIA_VENDOR_ID, 0x09ff, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2886946..8d6e13c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4411,8 +4411,12 @@ static struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8152)},
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8153)},
{REALTEK_USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101)},
- {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff)},
{}
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Lawrence Brakmo @ 2016-10-18 3:35 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <CAK6E8=cuuTY0u0xqSKmoVzAuCOMdCm2UhyFxAgpa=51bvh_u-A@mail.gmail.com>
Yuchung and Eric, thank you for your comments.
It looks like I need to think more about this patch. I was trying
to reduce the likelihood of reordering (which seems even more
important based on Eric¹s comment on pacing), but it seems like
the only way to prevent reordering is to only re-hash after an RTO
or when there are no packets in flight (which may not occur).
On 10/11/16, 8:56 PM, "Yuchung Cheng" <ycheng@google.com> wrote:
>On Tue, Oct 11, 2016 at 6:01 PM, Yuchung Cheng <ycheng@google.com> wrote:
>> On Tue, Oct 11, 2016 at 2:08 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>>> Yuchung, thank you for your comments. Responses inline.
>>>
>>> On 10/11/16, 12:49 PM, "Yuchung Cheng" <ycheng@google.com> wrote:
>>>
>>>>On Mon, Oct 10, 2016 at 5:18 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>>>>>
>>>>> The purpose of this patch is to help balance flows across paths. A
>>>>>new
>>>>> sysctl "tcp_retrans_txhash_prob" specifies the probability (0-100)
>>>>>that
>>>>> the txhash (IPv6 flowlabel) will be changed after a non-RTO
>>>>>retransmit.
>>>>> A probability is used in order to control how many flows are moved
>>>>> during a congestion event and prevent the congested path from
>>>>>becoming
>>>>> under utilized (which could occur if too many flows leave the current
>>>>> path). Txhash changes may be delayed in order to decrease the
>>>>>likelihood
>>>>> that it will trigger retransmists due to too much reordering.
>>>>>
>>>>> Another sysctl "tcp_retrans_txhash_mode" determines the behavior
>>>>>after
>>>>> RTOs. If the sysctl is 0, then after an RTO, only RTOs can trigger
>>>>> txhash changes. The idea is to decrease the likelihood of going back
>>>>> to a broken path. That is, we don't want flow balancing to trigger
>>>>> changes to broken paths. The drawback is that flow balancing does
>>>>> not work as well. If the sysctl is greater than 1, then we always
>>>>> do flow balancing, even after RTOs.
>>>>>
>>>>> Tested with packedrill tests (for correctness) and performance
>>>>> experiments with 2 and 3 paths. Performance experiments looked at
>>>>> aggregate goodput and fairness. For each run, we looked at the ratio
>>>>>of
>>>>> the goodputs for the fastest and slowest flows. These were averaged
>>>>>for
>>>>> all the runs. A fairness of 1 means all flows had the same goodput, a
>>>>> fairness of 2 means the fastest flow was twice as fast as the slowest
>>>>> flow.
>>>>>
>>>>> The setup for the performance experiments was 4 or 5 serves in a
>>>>>rack,
>>>>> 10G links. I tested various probabilities, but 20 seemed to have the
>>>>> best tradeoff for my setup (small RTTs).
>>>>>
>>>>> --- node1 -----
>>>>> sender --- switch --- node2 ----- switch ---- receiver
>>>>> --- node3 -----
>>>>>
>>>>> Scenario 1: One sender sends to one receiver through 2 routes (node1
>>>>>or
>>>>> node 2). The output from node1 and node2 is 1G (1gbit/sec). With
>>>>>only 2
>>>>> flows, without flow balancing (prob=0) the average goodput is 1.6G
>>>>>vs.
>>>>> 1.9G with flow balancing due to 2 flows ending up in one link and
>>>>>either
>>>>> not moving and taking some time to move. Fairness was 1 in all cases.
>>>>> For 7 flows, goodput was 1.9G for all, but fairness was 1.5, 1.4 or
>>>>>1.2
>>>>> for prob=0, prob=20,mode=0 and prob=20,mode=1 respectively. That is,
>>>>> flow balancing increased fairness.
>>>>>
>>>>> Scenario 2: One sender to one receiver, through 3 routes (node1,...
>>>>> node2). With 6 or 16 flows the goodput was the same for all, but
>>>>> fairness was 1.8, 1.5 and 1.2 respectively. Interestingly, the worst
>>>>> case fairness out of 10 runs were 2.2, 1.8 and 1.4 repectively. That
>>>>>is,
>>>>> prob=20,mode=1 improved average and worst case fairness.
>>>>I am wondering if we can build better API with routing layer to
>>>>implement this type of feature, instead of creeping the tx_rehashing
>>>>logic scatter in TCP. For example, we call dst_negative_advice on TCP
>>>>write timeouts.
>>>
>>> Not sure. The route is not necessarily bad, may be temporarily
>>>congested
>>> or they may all be congested. If all we want to do is change the txhash
>>> (unlike dst_negative_advice), then calling a tx_rehashing function may
>>> be the appropriate call.
>>>
>>>>
>>>>On the patch itself, it seems aggressive to (attempt to) rehash every
>>>>post-RTO retranmission. Also you can just use ca_state (==CA_Loss) to
>>>>identify post-RTO retransmission directly.
>>>
>>> Thanks, I will add the test.
>>>
>>>>
>>>>is this an implementation of the Flow Bender ?
>>>>https://urldefense.proofpoint.com/v2/url?u=http-3A__dl.acm.org_citation
>>>>.cf
>>>>m-3Fid-3D2674985&d=DQIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8ltkgyx
>>>>1u_
>>>>g&m=Q4nONH7kQ5AvQguw9UxpcHd79jfdDdrXj1YSJs7Ezhk&s=MA4fWBLMTGgRS0eGvBjxf
>>>>7BJ
>>>>Ol3-oxAzZDEYUG4cE-s&e=
>>>
>>> Part of flow bender, although there are also some similarities to
>>>flowlet
>>> switching.
>>>
>>>>
>>>>>
>>>>> Scenario 3: One sender to one receiver, 2 routes, one route drops
>>>>>50% of
>>>>> the packets. With 7 flows, goodput was the same 1.1G, but fairness
>>>>>was
>>>>> 1.8, 2.0 and 2.1 respectively. That is, if there is a bad route, then
>>>>> balancing, which does more re-routes, is less fair.
>>>>>
>>>>> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
>>>>> ---
>>>>> Documentation/networking/ip-sysctl.txt | 15 +++++++++++++++
>>>>> include/linux/tcp.h | 4 +++-
>>>>> include/net/tcp.h | 2 ++
>>>>> net/ipv4/sysctl_net_ipv4.c | 18 ++++++++++++++++++
>>>>> net/ipv4/tcp_input.c | 10 ++++++++++
>>>>> net/ipv4/tcp_output.c | 23 ++++++++++++++++++++++-
>>>>> net/ipv4/tcp_timer.c | 4 ++++
>>>>> 7 files changed, 74 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/networking/ip-sysctl.txt
>>>>>b/Documentation/networking/ip-sysctl.txt
>>>>> index 3db8c67..87a984c 100644
>>>>> --- a/Documentation/networking/ip-sysctl.txt
>>>>> +++ b/Documentation/networking/ip-sysctl.txt
>>>>> @@ -472,6 +472,21 @@ tcp_max_reordering - INTEGER
>>>>> if paths are using per packet load balancing (like bonding rr
>>>>>mode)
>>>>> Default: 300
>>>>>
>>>>> +tcp_retrans_txhash_mode - INTEGER
>>>>> + If zero, disable txhash recalculation due to non-RTO
>>>>>retransmissions
>>>>> + after an RTO. The idea is that broken paths will trigger an
>>>>>RTO
>>>>>and
>>>>> + we don't want going back to that path due to standard
>>>>>retransmissons
>>>>> + (flow balancing). The drawback is that balancing is less
>>>>>robust.
>>>>> + If greater than zero, can always (probabilistically)
>>>>>recalculate
>>>>> + txhash after non-RTO retransmissions.
>>>>> +
>>>>> +tcp_retrans_txhash_prob - INTEGER
>>>>> + Probability [0 to 100] that we will recalculate txhash when a
>>>>> + packet is resent not due to RTO (for RTO txhash is always
>>>>>recalculated).
>>>>> + The recalculation of the txhash may be delayed to decrease
>>>>>the
>>>>> + likelihood that reordering will trigger retransmissons.
>>>>> + The purpose is to help balance the flows among the possible
>>>>>paths.
>>>>> +
>>>>> tcp_retrans_collapse - BOOLEAN
>>>>> Bug-to-bug compatibility with some broken printers.
>>>>> On retransmit try to send bigger packets to work around bugs
>>>>>in
>>>>> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
>>>>> index a17ae7b..e0e3b7d 100644
>>>>> --- a/include/linux/tcp.h
>>>>> +++ b/include/linux/tcp.h
>>>>> @@ -214,7 +214,9 @@ struct tcp_sock {
>>>>> } rack;
>>>>> u16 advmss; /* Advertised MSS
>>>>>*/
>>>>> u8 rate_app_limited:1, /* rate_{delivered,interval_us}
>>>>>limited? */
>>>>> - unused:7;
>>>>> + txhash_rto:1, /* If set, don't do flow balancing
>>>>>*/
>>>>> + txhash_want:1, /* We want to change txhash when safe
>>>>>*/
>>>>> + unused:5;
>>>>> u8 nonagle : 4,/* Disable Nagle algorithm?
>>>>>*/
>>>>> thin_lto : 1,/* Use linear timeouts for thin
>>>>>streams
>>>>>*/
>>>>> thin_dupack : 1,/* Fast retransmit on first dupack
>>>>>*/
>>>>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>>>>> index f83b7f2..3abd304 100644
>>>>> --- a/include/net/tcp.h
>>>>> +++ b/include/net/tcp.h
>>>>> @@ -271,6 +271,8 @@ extern int sysctl_tcp_autocorking;
>>>>> extern int sysctl_tcp_invalid_ratelimit;
>>>>> extern int sysctl_tcp_pacing_ss_ratio;
>>>>> extern int sysctl_tcp_pacing_ca_ratio;
>>>>> +extern int sysctl_tcp_retrans_txhash_prob;
>>>>> +extern int sysctl_tcp_retrans_txhash_mode;
>>>>>
>>>>> extern atomic_long_t tcp_memory_allocated;
>>>>> extern struct percpu_counter tcp_sockets_allocated;
>>>>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
>>>>> index 1cb67de..00d6f26 100644
>>>>> --- a/net/ipv4/sysctl_net_ipv4.c
>>>>> +++ b/net/ipv4/sysctl_net_ipv4.c
>>>>> @@ -28,6 +28,7 @@
>>>>> static int zero;
>>>>> static int one = 1;
>>>>> static int four = 4;
>>>>> +static int hundred = 100;
>>>>> static int thousand = 1000;
>>>>> static int gso_max_segs = GSO_MAX_SEGS;
>>>>> static int tcp_retr1_max = 255;
>>>>> @@ -624,6 +625,23 @@ static struct ctl_table ipv4_table[] = {
>>>>> .proc_handler = proc_dointvec_ms_jiffies,
>>>>> },
>>>>> {
>>>>> + .procname = "tcp_retrans_txhash_prob",
>>>>> + .data = &sysctl_tcp_retrans_txhash_prob,
>>>>> + .maxlen = sizeof(int),
>>>>> + .mode = 0644,
>>>>> + .proc_handler = proc_dointvec_minmax,
>>>>> + .extra1 = &zero,
>>>>> + .extra2 = &hundred,
>>>>> + },
>>>>> + {
>>>>> + .procname = "tcp_retrans_txhash_mode",
>>>>> + .data = &sysctl_tcp_retrans_txhash_mode,
>>>>> + .maxlen = sizeof(int),
>>>>> + .mode = 0644,
>>>>> + .proc_handler = proc_dointvec_minmax,
>>>>> + .extra1 = &zero,
>>>>> + },
>>>>> + {
>>>>> .procname = "icmp_msgs_per_sec",
>>>>> .data = &sysctl_icmp_msgs_per_sec,
>>>>> .maxlen = sizeof(int),
>>>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>>>> index a27b9c0..fed5366 100644
>>>>> --- a/net/ipv4/tcp_input.c
>>>>> +++ b/net/ipv4/tcp_input.c
>>>>> @@ -101,6 +101,9 @@ int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
>>>>> int sysctl_tcp_early_retrans __read_mostly = 3;
>>>>> int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
>>>>>
>>>>> +int sysctl_tcp_retrans_txhash_prob __read_mostly;
>>>>> +int sysctl_tcp_retrans_txhash_mode __read_mostly;
>>>>> +
>>>>> #define FLAG_DATA 0x01 /* Incoming frame contained
>>>>>data.
>>>>> */
>>>>> #define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a
>>>>>window update. */
>>>>> #define FLAG_DATA_ACKED 0x04 /* This ACK acknowledged
>>>>>new data. */
>>>>> @@ -3674,6 +3677,13 @@ static int tcp_ack(struct sock *sk, const
>>>>>struct
>>>>>sk_buff *skb, int flag)
>>>>> flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una,
>>>>>&acked,
>>>>> &sack_state, &now);
>>>>>
>>>>> + /* Check if we should set txhash (would not cause
>>>>>reordering) */
>>>>> + if (tp->txhash_want &&
>>>>> + (tp->packets_out - tp->sacked_out) < tp->reordering) {
>>>>> + sk_set_txhash(sk);
>>>>> + tp->txhash_want = 0;
>>>>> + }
>>>>> +
>>>>> if (tcp_ack_is_dubious(sk, flag)) {
>>>>> is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED |
>>>>>FLAG_NOT_DUP));
>>>>> tcp_fastretrans_alert(sk, acked, is_dupack, &flag,
>>>>>&rexmit);
>>>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>>>> index 896e9df..58490ac 100644
>>>>> --- a/net/ipv4/tcp_output.c
>>>>> +++ b/net/ipv4/tcp_output.c
>>>>> @@ -2738,9 +2738,30 @@ int tcp_retransmit_skb(struct sock *sk, struct
>>>>>sk_buff *skb, int segs)
>>>>> tp->retrans_out += tcp_skb_pcount(skb);
>>>>>
>>>>> /* Save stamp of the first retransmit. */
>>>>> - if (!tp->retrans_stamp)
>>>>> + if (!tp->retrans_stamp) {
>>>>> tp->retrans_stamp = tcp_skb_timestamp(skb);
>>>>>
>>>>> + /* Determine if we should reset hash, only
>>>>>done
>>>>>once
>>>>> + * per recovery
>>>>> + */
>>>>> + if ((!tp->txhash_rto ||
>>>>> + sysctl_tcp_retrans_txhash_mode > 0) &&
>>>>> + sk->sk_txhash &&
>>>>> + (prandom_u32_max(100) <
>>>>> + sysctl_tcp_retrans_txhash_prob)) {
>>>>> + /* If not too much reordering, or
>>>>>RTT is
>>>>> + * small enough that we don't care
>>>>>about
>>>>> + * reordering, then change it now.
>>>>> + * Else, wait until it is safe.
>>>>> + */
>>>>> + if ((tp->packets_out -
>>>>>tp->sacked_out) <
>>>>> + tp->reordering)
>>>>I don't parse this logic ... suppose reordering is 100 (not uncommon
>>>>today due to the last packet being delivered slightly earlier than the
>>>>rest), and cwnd==packets_out =~200,we only want to rehash until half
>>>>of the packets are sacked, so we are still rehashing even when
>>>>reordering is heavy?
>>>
>>> In your scenario, there would be no re-hashing until sacked_out is 101
>>> ((packets_out - sacked_out) < 100). This code would mark txhash_want.
>>> Then when an ACK is received and the conditional is true, txhash
>>> would be changed.
>>>
>>> Now, the test does not prevent retransmissions due to reordering in all
>>> cases, but hopefully in most. I will also add the test you recommended,
>>> checking for CA_Loss, to prevent too much re-hashing.
>> If the whole point is to rehash at most once between recovery events,
>> why do we need this complicated change at per packet level in
>> tcp_retransmit_skb()? there are functions that start and end recovery
>> (tcp_enter_recovery, tcp_end_cwnd_reduction). our retransmission logic
>> is already very complicated.
>>
>> I still don't understand the incentive of starting the rehashing
>> half-way retransmitting depending on the sacking and reordering
>> status, for temporarily congestion as you've mentioned.
>>
>> is this feature unique for intra-DC connections?
>I thought more about this patch on my way home and have more
>questions: why do we exclude RTO retransmission specifically? also
>when we rehash, we'll introduce reordering either in recovery or after
>recovery, as some TCP CC like bbr would continue sending regardlessly,
>so starting in tcp_ack() with tp->txhash_want does not really prevent
>causing more reordering.
>
>so if we want to rehash upon a recovery event (fast or timeout), then
>we can make a helper function ie tcp_retry_alt_path, and just call it
>in tcp_enter_loss and tcp_recovery (which are called once per
>recovery). Since it'll cause reordering anyways, I doubt we need to
>check tp->reordering at all. If packets already traverse different
>paths, rehashing can make it better or worse. if packets weren't
>traversing different paths, the rehashing itself can introduce
>reordering. so in the end there is much benefit to make decision based
>on current inflight and reordering IMO...
>
>
>>
>>>
>>>>
>>>>also where do we check RTT is small?
>>>
>>> The RTT comment is left over from a previous version, I will remove it.
>>>
>>>>
>>>>> + sk_set_txhash(sk);
>>>>> + else
>>>>> + tp->txhash_want = 1;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> } else if (err != -EBUSY) {
>>>>> NET_INC_STATS(sock_net(sk),
>>>>>LINUX_MIB_TCPRETRANSFAIL);
>>>>> }
>>>>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>>>>> index 3ea1cf8..e66baad 100644
>>>>> --- a/net/ipv4/tcp_timer.c
>>>>> +++ b/net/ipv4/tcp_timer.c
>>>>> @@ -186,6 +186,8 @@ static int tcp_write_timeout(struct sock *sk)
>>>>>
>>>>> if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
>>>>> if (icsk->icsk_retransmits) {
>>>>> + tp->txhash_rto = 1;
>>>>> + tp->txhash_want = 0;
>>>>> dst_negative_advice(sk);
>>>>> if (tp->syn_fastopen || tp->syn_data)
>>>>> tcp_fastopen_cache_set(sk, 0, NULL,
>>>>>true, 0);
>>>>> @@ -218,6 +220,8 @@ static int tcp_write_timeout(struct sock *sk)
>>>>> } else {
>>>>> sk_rethink_txhash(sk);
>>>>> }
>>>>> + tp->txhash_rto = 1;
>>>>> + tp->txhash_want = 0;
>>>>>
>>>>> retry_until = net->ipv4.sysctl_tcp_retries2;
>>>>> if (sock_flag(sk, SOCK_DEAD)) {
>>>>> --
>>>>> 2.9.3
>>>>>
>>>
^ permalink raw reply
* [PATCH net-next 07/11] mlxsw: Flip to the new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri, netdev, davem
Cc: dledford, sean.hefty, hal.rosenstock, linux-rdma, j.vosburgh,
vfalico, andy, jeffrey.t.kirsher, intel-wired-lan, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa@cumulusnetworks.com>
Convert mlxsw users to new dev walk API. This is just a code conversion;
no functional change is intended.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 37 ++++++++++++++++----------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 43a5eddc2c11..99805fd3d110 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3092,19 +3092,30 @@ static bool mlxsw_sp_port_dev_check(const struct net_device *dev)
return dev->netdev_ops == &mlxsw_sp_port_netdev_ops;
}
+static int mlxsw_lower_dev_walk(struct net_device *lower_dev, void *data)
+{
+ struct mlxsw_sp_port **port = data;
+ int ret = 0;
+
+ if (mlxsw_sp_port_dev_check(lower_dev)) {
+ *port = netdev_priv(lower_dev);
+ ret = 1;
+ }
+
+ return ret;
+}
+
static struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find(struct net_device *dev)
{
- struct net_device *lower_dev;
- struct list_head *iter;
+ struct mlxsw_sp_port *port;
if (mlxsw_sp_port_dev_check(dev))
return netdev_priv(dev);
- netdev_for_each_all_lower_dev(dev, lower_dev, iter) {
- if (mlxsw_sp_port_dev_check(lower_dev))
- return netdev_priv(lower_dev);
- }
- return NULL;
+ port = NULL;
+ netdev_walk_all_lower_dev(dev, mlxsw_lower_dev_walk, &port);
+
+ return port;
}
static struct mlxsw_sp *mlxsw_sp_lower_get(struct net_device *dev)
@@ -3117,17 +3128,15 @@ static struct mlxsw_sp *mlxsw_sp_lower_get(struct net_device *dev)
static struct mlxsw_sp_port *mlxsw_sp_port_dev_lower_find_rcu(struct net_device *dev)
{
- struct net_device *lower_dev;
- struct list_head *iter;
+ struct mlxsw_sp_port *port;
if (mlxsw_sp_port_dev_check(dev))
return netdev_priv(dev);
- netdev_for_each_all_lower_dev_rcu(dev, lower_dev, iter) {
- if (mlxsw_sp_port_dev_check(lower_dev))
- return netdev_priv(lower_dev);
- }
- return NULL;
+ port = NULL;
+ netdev_walk_all_lower_dev_rcu(dev, mlxsw_lower_dev_walk, &port);
+
+ return port;
}
struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev)
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 05/11] IB/ipoib: Flip to new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri, netdev, davem
Cc: dledford, sean.hefty, hal.rosenstock, linux-rdma, j.vosburgh,
vfalico, andy, jeffrey.t.kirsher, intel-wired-lan, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa@cumulusnetworks.com>
Convert ipoib_get_net_dev_match_addr to the new upper device walk API.
This is just a code conversion; no functional change is intended.
v2
- removed typecast of data
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 37 +++++++++++++++++++++----------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 5636fc3da6b8..cc059218c962 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -292,6 +292,25 @@ static struct net_device *ipoib_get_master_net_dev(struct net_device *dev)
return dev;
}
+struct ipoib_walk_data {
+ const struct sockaddr *addr;
+ struct net_device *result;
+};
+
+static int ipoib_upper_walk(struct net_device *upper, void *_data)
+{
+ struct ipoib_walk_data *data = _data;
+ int ret = 0;
+
+ if (ipoib_is_dev_match_addr_rcu(data->addr, upper)) {
+ dev_hold(upper);
+ data->result = upper;
+ ret = 1;
+ }
+
+ return ret;
+}
+
/**
* Find a net_device matching the given address, which is an upper device of
* the given net_device.
@@ -304,27 +323,21 @@ static struct net_device *ipoib_get_master_net_dev(struct net_device *dev)
static struct net_device *ipoib_get_net_dev_match_addr(
const struct sockaddr *addr, struct net_device *dev)
{
- struct net_device *upper,
- *result = NULL;
- struct list_head *iter;
+ struct ipoib_walk_data data = {
+ .addr = addr,
+ };
rcu_read_lock();
if (ipoib_is_dev_match_addr_rcu(addr, dev)) {
dev_hold(dev);
- result = dev;
+ data.result = dev;
goto out;
}
- netdev_for_each_all_upper_dev_rcu(dev, upper, iter) {
- if (ipoib_is_dev_match_addr_rcu(addr, upper)) {
- dev_hold(upper);
- result = upper;
- break;
- }
- }
+ netdev_walk_all_upper_dev_rcu(dev, ipoib_upper_walk, &data);
out:
rcu_read_unlock();
- return result;
+ return data.result;
}
/* returns the number of IPoIB netdevs on top a given ipoib device matching a
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri, netdev, davem
Cc: dledford, sean.hefty, hal.rosenstock, linux-rdma, j.vosburgh,
vfalico, andy, jeffrey.t.kirsher, intel-wired-lan, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa@cumulusnetworks.com>
This patch introduces netdev_walk_all_upper_dev_rcu,
netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These
functions recursively walk the adj_list of devices to determine all upper
and lower devices.
The functions take a callback function that is invoked for each device
in the list. If the callback returns non-0, the walk is terminated and
the functions return that code back to callers.
v3
- simplified netdev_has_upper_dev_all_rcu and __netdev_has_upper_dev and
removed typecast as suggested by Stephen
v2
- fixed definition of netdev_next_lower_dev_rcu to mirror the upper_dev
version.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
include/linux/netdevice.h | 17 +++++
net/core/dev.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 172 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bf341b65ca5e..a5902d995907 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3778,6 +3778,14 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
updev; \
updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
+int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
+ int (*fn)(struct net_device *upper_dev,
+ void *data),
+ void *data);
+
+bool netdev_has_upper_dev_all_rcu(struct net_device *dev,
+ struct net_device *upper_dev);
+
void *netdev_lower_get_next_private(struct net_device *dev,
struct list_head **iter);
void *netdev_lower_get_next_private_rcu(struct net_device *dev,
@@ -3821,6 +3829,15 @@ struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev,
ldev; \
ldev = netdev_all_lower_get_next_rcu(dev, &(iter)))
+int netdev_walk_all_lower_dev(struct net_device *dev,
+ int (*fn)(struct net_device *lower_dev,
+ void *data),
+ void *data);
+int netdev_walk_all_lower_dev_rcu(struct net_device *dev,
+ int (*fn)(struct net_device *lower_dev,
+ void *data),
+ void *data);
+
void *netdev_adjacent_get_private(struct list_head *adj_list);
void *netdev_lower_get_first_private_rcu(struct net_device *dev);
struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index f67fd16615bb..fc48337cfab8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5156,6 +5156,31 @@ bool netdev_has_upper_dev(struct net_device *dev,
EXPORT_SYMBOL(netdev_has_upper_dev);
/**
+ * netdev_has_upper_dev_all - Check if device is linked to an upper device
+ * @dev: device
+ * @upper_dev: upper device to check
+ *
+ * Find out if a device is linked to specified upper device and return true
+ * in case it is. Note that this checks the entire upper device chain.
+ * The caller must hold rcu lock.
+ */
+
+static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data)
+{
+ struct net_device *dev = data;
+
+ return upper_dev == dev;
+}
+
+bool netdev_has_upper_dev_all_rcu(struct net_device *dev,
+ struct net_device *upper_dev)
+{
+ return !!netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev,
+ upper_dev);
+}
+EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu);
+
+/**
* netdev_has_any_upper_dev - Check if device is linked to some device
* @dev: device
*
@@ -5255,6 +5280,51 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
+static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *upper;
+
+ WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held());
+
+ upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
+
+ if (&upper->list == &dev->adj_list.upper)
+ return NULL;
+
+ *iter = &upper->list;
+
+ return upper->dev;
+}
+
+int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
+ int (*fn)(struct net_device *dev,
+ void *data),
+ void *data)
+{
+ struct net_device *udev;
+ struct list_head *iter;
+ int ret;
+
+ for (iter = &dev->adj_list.upper,
+ udev = netdev_next_upper_dev_rcu(dev, &iter);
+ udev;
+ udev = netdev_next_upper_dev_rcu(dev, &iter)) {
+ /* first is the upper device itself */
+ ret = fn(udev, data);
+ if (ret)
+ return ret;
+
+ /* then look at all of its upper devices */
+ ret = netdev_walk_all_upper_dev_rcu(udev, fn, data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(netdev_walk_all_upper_dev_rcu);
+
/**
* netdev_lower_get_next_private - Get the next ->private from the
* lower neighbour list
@@ -5361,6 +5431,49 @@ struct net_device *netdev_all_lower_get_next(struct net_device *dev, struct list
}
EXPORT_SYMBOL(netdev_all_lower_get_next);
+static struct net_device *netdev_next_lower_dev(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *lower;
+
+ lower = list_entry(*iter, struct netdev_adjacent, list);
+
+ if (&lower->list == &dev->adj_list.lower)
+ return NULL;
+
+ *iter = lower->list.next;
+
+ return lower->dev;
+}
+
+int netdev_walk_all_lower_dev(struct net_device *dev,
+ int (*fn)(struct net_device *dev,
+ void *data),
+ void *data)
+{
+ struct net_device *ldev;
+ struct list_head *iter;
+ int ret;
+
+ for (iter = &dev->adj_list.lower,
+ ldev = netdev_next_lower_dev(dev, &iter);
+ ldev;
+ ldev = netdev_next_lower_dev(dev, &iter)) {
+ /* first is the lower device itself */
+ ret = fn(ldev, data);
+ if (ret)
+ return ret;
+
+ /* then look at all of its lower devices */
+ ret = netdev_walk_all_lower_dev(ldev, fn, data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev);
+
/**
* netdev_all_lower_get_next_rcu - Get the next device from all
* lower neighbour list, RCU variant
@@ -5382,6 +5495,48 @@ struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_all_lower_get_next_rcu);
+static struct net_device *netdev_next_lower_dev_rcu(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *lower;
+
+ lower = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
+ if (&lower->list == &dev->adj_list.lower)
+ return NULL;
+
+ *iter = &lower->list;
+
+ return lower->dev;
+}
+
+int netdev_walk_all_lower_dev_rcu(struct net_device *dev,
+ int (*fn)(struct net_device *dev,
+ void *data),
+ void *data)
+{
+ struct net_device *ldev;
+ struct list_head *iter;
+ int ret;
+
+ for (iter = &dev->adj_list.lower,
+ ldev = netdev_next_lower_dev_rcu(dev, &iter);
+ ldev;
+ ldev = netdev_next_lower_dev_rcu(dev, &iter)) {
+ /* first is the lower device itself */
+ ret = fn(ldev, data);
+ if (ret)
+ return ret;
+
+ /* then look at all of its lower devices */
+ ret = netdev_walk_all_lower_dev_rcu(ldev, fn, data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev_rcu);
+
/**
* netdev_lower_get_first_private_rcu - Get the first ->private from the
* lower neighbour list, RCU
--
2.1.4
^ permalink raw reply related
* [PATCH v3 net-next 00/11] net: Fix netdev adjacency tracking
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri, netdev, davem
Cc: dledford, sean.hefty, hal.rosenstock, linux-rdma, j.vosburgh,
vfalico, andy, jeffrey.t.kirsher, intel-wired-lan, David Ahern
The netdev adjacency tracking is failing to create proper dependencies
for some topologies. For example this topology
+--------+
| myvrf |
+--------+
| |
| +---------+
| | macvlan |
| +---------+
| |
+----------+
| bridge |
+----------+
|
+--------+
| bond1 |
+--------+
|
+--------+
| eth3 |
+--------+
hits 1 of 2 problems depending on the order of enslavement. The base set of
commands for both cases:
ip link add bond1 type bond
ip link set bond1 up
ip link set eth3 down
ip link set eth3 master bond1
ip link set eth3 up
ip link add bridge type bridge
ip link set bridge up
ip link add macvlan link bridge type macvlan
ip link set macvlan up
ip link add myvrf type vrf table 1234
ip link set myvrf up
ip link set bridge master myvrf
Case 1 enslave macvlan to the vrf before enslaving the bond to the bridge:
ip link set macvlan master myvrf
ip link set bond1 master bridge
Attempts to delete the VRF:
ip link delete myvrf
trigger the BUG in __netdev_adjacent_dev_remove:
[ 587.405260] tried to remove device eth3 from myvrf
[ 587.407269] ------------[ cut here ]------------
[ 587.408918] kernel BUG at /home/dsa/kernel.git/net/core/dev.c:5661!
[ 587.411113] invalid opcode: 0000 [#1] SMP
[ 587.412454] Modules linked in: macvlan bridge stp llc bonding vrf
[ 587.414765] CPU: 0 PID: 726 Comm: ip Not tainted 4.8.0+ #109
[ 587.416766] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 587.420241] task: ffff88013ab6eec0 task.stack: ffffc90000628000
[ 587.422163] RIP: 0010:[<ffffffff813cef03>] [<ffffffff813cef03>] __netdev_adjacent_dev_remove+0x40/0x12c
...
[ 587.446053] Call Trace:
[ 587.446424] [<ffffffff813d1542>] __netdev_adjacent_dev_unlink+0x20/0x3c
[ 587.447390] [<ffffffff813d16a3>] netdev_upper_dev_unlink+0xfa/0x15e
[ 587.448297] [<ffffffffa00003a3>] vrf_del_slave+0x13/0x2a [vrf]
[ 587.449153] [<ffffffffa00004a4>] vrf_dev_uninit+0xea/0x114 [vrf]
[ 587.450036] [<ffffffff813d19b0>] rollback_registered_many+0x22b/0x2da
[ 587.450974] [<ffffffff813d1aac>] unregister_netdevice_many+0x17/0x48
[ 587.451903] [<ffffffff813de444>] rtnl_delete_link+0x3c/0x43
[ 587.452719] [<ffffffff813dedcd>] rtnl_dellink+0x180/0x194
When the BUG is converted to a WARN_ON it shows 4 missing adjacencies:
eth3 - myvrf, mvrf - eth3, bond1 - myvrf and myvrf - bond1
All of those are because the __netdev_upper_dev_link function does not
properly link macvlan lower devices to myvrf when it is enslaved.
The second case just flips the ordering of the enslavements:
ip link set bond1 master bridge
ip link set macvlan master myvrf
Then run:
ip link delete bond1
ip link delete myvrf
The vrf delete command hangs because myvrf has a reference that has not
been released. In this case the removal code does not account for 2 paths
between eth3 and myvrf - one from bridge to vrf and the other through the
macvlan.
Rather than try to maintain a linked list of all upper and lower devices
per netdevice, only track the direct neighbors. The remaining stack can
be determined by recursively walking the neighbors.
The existing netdev_for_each_all_upper_dev_rcu,
netdev_for_each_all_lower_dev and netdev_for_each_all_lower_dev_rcu macros
are replaced with APIs that walk the upper and lower device lists. The
new APIs take a callback function and a data arg that is passed to the
callback for each device in the list. Drivers using the old macros are
converted in separate patches to make it easier on reviewers. It is an
API conversion only; no functional change is intended.
v3
- address Stephen's comment to simplify logic and remove typecasts
v2
- fixed bond0 references in cover-letter
- fixed definition of netdev_next_lower_dev_rcu to mirror the upper_dev
version.
David Ahern (11):
net: Remove refnr arg when inserting link adjacencies
net: Introduce new api for walking upper and lower devices
net: bonding: Flip to the new dev walk API
IB/core: Flip to the new dev walk API
IB/ipoib: Flip to new dev walk API
ixgbe: Flip to the new dev walk API
mlxsw: Flip to the new dev walk API
rocker: Flip to the new dev walk API
net: Remove all_adj_list and its references
net: Add warning if any lower device is still in adjacency list
net: dev: Improve debug statements for adjacency tracking
drivers/infiniband/core/core_priv.h | 9 +-
drivers/infiniband/core/roce_gid_mgmt.c | 42 +--
drivers/infiniband/ulp/ipoib/ipoib_main.c | 37 ++-
drivers/net/bonding/bond_alb.c | 82 +++---
drivers/net/bonding/bond_main.c | 17 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 132 ++++++----
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 37 ++-
drivers/net/ethernet/rocker/rocker_main.c | 31 ++-
include/linux/netdevice.h | 38 ++-
net/core/dev.c | 350 ++++++++++++-------------
10 files changed, 423 insertions(+), 352 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH net-next 11/11] net: dev: Improve debug statements for adjacency tracking
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Adjacency code only has debugs for the insert case. Add debugs for
the remove path and make both consistently worded to make it easier
to follow the insert and removal with reference counts.
In addition, change the BUG to a WARN_ON. A missing adjacency at
removal time is not cause for a panic.
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
net/core/dev.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index c6bbf310d407..f55fb4536016 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5561,6 +5561,9 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
if (adj) {
adj->ref_nr += 1;
+ pr_debug("Insert adjacency: dev %s adj_dev %s adj->ref_nr %d\n",
+ dev->name, adj_dev->name, adj->ref_nr);
+
return 0;
}
@@ -5574,8 +5577,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj->private = private;
dev_hold(adj_dev);
- pr_debug("dev_hold for %s, because of link added from %s to %s\n",
- adj_dev->name, dev->name, adj_dev->name);
+ pr_debug("Insert adjacency: dev %s adj_dev %s adj->ref_nr %d; dev_hold on %s\n",
+ dev->name, adj_dev->name, adj->ref_nr, adj_dev->name);
if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) {
ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list);
@@ -5614,17 +5617,22 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
{
struct netdev_adjacent *adj;
+ pr_debug("Remove adjacency: dev %s adj_dev %s ref_nr %d\n",
+ dev->name, adj_dev->name, ref_nr);
+
adj = __netdev_find_adj(adj_dev, dev_list);
if (!adj) {
- pr_err("tried to remove device %s from %s\n",
+ pr_err("Adjacency does not exist for device %s from %s\n",
dev->name, adj_dev->name);
- BUG();
+ WARN_ON(1);
+ return;
}
if (adj->ref_nr > ref_nr) {
- pr_debug("%s to %s ref_nr-%d = %d\n", dev->name, adj_dev->name,
- ref_nr, adj->ref_nr-ref_nr);
+ pr_debug("adjacency: %s to %s ref_nr - %d = %d\n",
+ dev->name, adj_dev->name, ref_nr,
+ adj->ref_nr - ref_nr);
adj->ref_nr -= ref_nr;
return;
}
@@ -5636,7 +5644,7 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list);
list_del_rcu(&adj->list);
- pr_debug("dev_put for %s, because link removed from %s to %s\n",
+ pr_debug("adjacency: dev_put for %s, because link removed from %s to %s\n",
adj_dev->name, dev->name, adj_dev->name);
dev_put(adj_dev);
kfree_rcu(adj, rcu);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 10/11] net: Add warning if any lower device is still in adjacency list
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Lower list should be empty just like upper.
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
net/core/dev.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index a9fe14908b44..c6bbf310d407 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5219,6 +5219,20 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
}
EXPORT_SYMBOL(netdev_master_upper_dev_get);
+/**
+ * netdev_has_any_lower_dev - Check if device is linked to some device
+ * @dev: device
+ *
+ * Find out if a device is linked to a lower device and return true in case
+ * it is. The caller must hold the RTNL lock.
+ */
+static bool netdev_has_any_lower_dev(struct net_device *dev)
+{
+ ASSERT_RTNL();
+
+ return !list_empty(&dev->adj_list.lower);
+}
+
void *netdev_adjacent_get_private(struct list_head *adj_list)
{
struct netdev_adjacent *adj;
@@ -6616,6 +6630,7 @@ static void rollback_registered_many(struct list_head *head)
/* Notifier chain MUST detach us all upper devices. */
WARN_ON(netdev_has_any_upper_dev(dev));
+ WARN_ON(netdev_has_any_lower_dev(dev));
/* Remove entries from kobject tree */
netdev_unregister_kobject(dev);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 09/11] net: Remove all_adj_list and its references
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Only direct adjacencies are maintained. All upper or lower devices can
be learned via the new walk API which recursively walks the adj_list for
upper devices or lower devices.
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
include/linux/netdevice.h | 25 ------
net/core/dev.c | 223 ++++------------------------------------------
2 files changed, 18 insertions(+), 230 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a5902d995907..458c87631e7f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1456,7 +1456,6 @@ enum netdev_priv_flags {
* @ptype_specific: Device-specific, protocol-specific packet handlers
*
* @adj_list: Directly linked devices, like slaves for bonding
- * @all_adj_list: All linked devices, *including* neighbours
* @features: Currently active device features
* @hw_features: User-changeable features
*
@@ -1675,11 +1674,6 @@ struct net_device {
struct list_head lower;
} adj_list;
- struct {
- struct list_head upper;
- struct list_head lower;
- } all_adj_list;
-
netdev_features_t features;
netdev_features_t hw_features;
netdev_features_t wanted_features;
@@ -3771,13 +3765,6 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
updev; \
updev = netdev_upper_get_next_dev_rcu(dev, &(iter)))
-/* iterate through upper list, must be called under RCU read lock */
-#define netdev_for_each_all_upper_dev_rcu(dev, updev, iter) \
- for (iter = &(dev)->all_adj_list.upper, \
- updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)); \
- updev; \
- updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
-
int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
int (*fn)(struct net_device *upper_dev,
void *data),
@@ -3817,18 +3804,6 @@ struct net_device *netdev_all_lower_get_next(struct net_device *dev,
struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev,
struct list_head **iter);
-#define netdev_for_each_all_lower_dev(dev, ldev, iter) \
- for (iter = (dev)->all_adj_list.lower.next, \
- ldev = netdev_all_lower_get_next(dev, &(iter)); \
- ldev; \
- ldev = netdev_all_lower_get_next(dev, &(iter)))
-
-#define netdev_for_each_all_lower_dev_rcu(dev, ldev, iter) \
- for (iter = (dev)->all_adj_list.lower.next, \
- ldev = netdev_all_lower_get_next_rcu(dev, &(iter)); \
- ldev; \
- ldev = netdev_all_lower_get_next_rcu(dev, &(iter)))
-
int netdev_walk_all_lower_dev(struct net_device *dev,
int (*fn)(struct net_device *lower_dev,
void *data),
diff --git a/net/core/dev.c b/net/core/dev.c
index fc48337cfab8..a9fe14908b44 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5137,6 +5137,13 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *adj_dev,
return NULL;
}
+static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data)
+{
+ struct net_device *dev = data;
+
+ return upper_dev == dev;
+}
+
/**
* netdev_has_upper_dev - Check if device is linked to an upper device
* @dev: device
@@ -5151,7 +5158,8 @@ bool netdev_has_upper_dev(struct net_device *dev,
{
ASSERT_RTNL();
- return __netdev_find_adj(upper_dev, &dev->all_adj_list.upper);
+ return netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev,
+ upper_dev);
}
EXPORT_SYMBOL(netdev_has_upper_dev);
@@ -5165,13 +5173,6 @@ EXPORT_SYMBOL(netdev_has_upper_dev);
* The caller must hold rcu lock.
*/
-static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data)
-{
- struct net_device *dev = data;
-
- return upper_dev == dev;
-}
-
bool netdev_has_upper_dev_all_rcu(struct net_device *dev,
struct net_device *upper_dev)
{
@@ -5191,7 +5192,7 @@ static bool netdev_has_any_upper_dev(struct net_device *dev)
{
ASSERT_RTNL();
- return !list_empty(&dev->all_adj_list.upper);
+ return !list_empty(&dev->adj_list.upper);
}
/**
@@ -5254,32 +5255,6 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_upper_get_next_dev_rcu);
-/**
- * netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
- * @dev: device
- * @iter: list_head ** of the current position
- *
- * Gets the next device from the dev's upper list, starting from iter
- * position. The caller must hold RCU read lock.
- */
-struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
- struct list_head **iter)
-{
- struct netdev_adjacent *upper;
-
- WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held());
-
- upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
-
- if (&upper->list == &dev->all_adj_list.upper)
- return NULL;
-
- *iter = &upper->list;
-
- return upper->dev;
-}
-EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
-
static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev,
struct list_head **iter)
{
@@ -5406,31 +5381,6 @@ void *netdev_lower_get_next(struct net_device *dev, struct list_head **iter)
}
EXPORT_SYMBOL(netdev_lower_get_next);
-/**
- * netdev_all_lower_get_next - Get the next device from all lower neighbour list
- * @dev: device
- * @iter: list_head ** of the current position
- *
- * Gets the next netdev_adjacent from the dev's all lower neighbour
- * list, starting from iter position. The caller must hold RTNL lock or
- * its own locking that guarantees that the neighbour all lower
- * list will remain unchanged.
- */
-struct net_device *netdev_all_lower_get_next(struct net_device *dev, struct list_head **iter)
-{
- struct netdev_adjacent *lower;
-
- lower = list_entry(*iter, struct netdev_adjacent, list);
-
- if (&lower->list == &dev->all_adj_list.lower)
- return NULL;
-
- *iter = lower->list.next;
-
- return lower->dev;
-}
-EXPORT_SYMBOL(netdev_all_lower_get_next);
-
static struct net_device *netdev_next_lower_dev(struct net_device *dev,
struct list_head **iter)
{
@@ -5474,27 +5424,6 @@ int netdev_walk_all_lower_dev(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev);
-/**
- * netdev_all_lower_get_next_rcu - Get the next device from all
- * lower neighbour list, RCU variant
- * @dev: device
- * @iter: list_head ** of the current position
- *
- * Gets the next netdev_adjacent from the dev's all lower neighbour
- * list, starting from iter position. The caller must hold RCU read lock.
- */
-struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev,
- struct list_head **iter)
-{
- struct netdev_adjacent *lower;
-
- lower = list_first_or_null_rcu(&dev->all_adj_list.lower,
- struct netdev_adjacent, list);
-
- return lower ? lower->dev : NULL;
-}
-EXPORT_SYMBOL(netdev_all_lower_get_next_rcu);
-
static struct net_device *netdev_next_lower_dev_rcu(struct net_device *dev,
struct list_head **iter)
{
@@ -5722,15 +5651,6 @@ static int __netdev_adjacent_dev_link_lists(struct net_device *dev,
return 0;
}
-static int __netdev_adjacent_dev_link(struct net_device *dev,
- struct net_device *upper_dev)
-{
- return __netdev_adjacent_dev_link_lists(dev, upper_dev,
- &dev->all_adj_list.upper,
- &upper_dev->all_adj_list.lower,
- NULL, false);
-}
-
static void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
struct net_device *upper_dev,
u16 ref_nr,
@@ -5741,40 +5661,19 @@ static void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
__netdev_adjacent_dev_remove(upper_dev, dev, ref_nr, down_list);
}
-static void __netdev_adjacent_dev_unlink(struct net_device *dev,
- struct net_device *upper_dev,
- u16 ref_nr)
-{
- __netdev_adjacent_dev_unlink_lists(dev, upper_dev, ref_nr,
- &dev->all_adj_list.upper,
- &upper_dev->all_adj_list.lower);
-}
-
static int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
struct net_device *upper_dev,
void *private, bool master)
{
- int ret = __netdev_adjacent_dev_link(dev, upper_dev);
-
- if (ret)
- return ret;
-
- ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
- &dev->adj_list.upper,
- &upper_dev->adj_list.lower,
- private, master);
- if (ret) {
- __netdev_adjacent_dev_unlink(dev, upper_dev, 1);
- return ret;
- }
-
- return 0;
+ return __netdev_adjacent_dev_link_lists(dev, upper_dev,
+ &dev->adj_list.upper,
+ &upper_dev->adj_list.lower,
+ private, master);
}
static void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
struct net_device *upper_dev)
{
- __netdev_adjacent_dev_unlink(dev, upper_dev, 1);
__netdev_adjacent_dev_unlink_lists(dev, upper_dev, 1,
&dev->adj_list.upper,
&upper_dev->adj_list.lower);
@@ -5785,7 +5684,6 @@ static int __netdev_upper_dev_link(struct net_device *dev,
void *upper_priv, void *upper_info)
{
struct netdev_notifier_changeupper_info changeupper_info;
- struct netdev_adjacent *i, *j, *to_i, *to_j;
int ret = 0;
ASSERT_RTNL();
@@ -5794,10 +5692,10 @@ static int __netdev_upper_dev_link(struct net_device *dev,
return -EBUSY;
/* To prevent loops, check if dev is not upper device to upper_dev. */
- if (__netdev_find_adj(dev, &upper_dev->all_adj_list.upper))
+ if (netdev_has_upper_dev(upper_dev, dev))
return -EBUSY;
- if (__netdev_find_adj(upper_dev, &dev->adj_list.upper))
+ if (netdev_has_upper_dev(dev, upper_dev))
return -EEXIST;
if (master && netdev_master_upper_dev_get(dev))
@@ -5819,80 +5717,15 @@ static int __netdev_upper_dev_link(struct net_device *dev,
if (ret)
return ret;
- /* Now that we linked these devs, make all the upper_dev's
- * all_adj_list.upper visible to every dev's all_adj_list.lower an
- * versa, and don't forget the devices itself. All of these
- * links are non-neighbours.
- */
- list_for_each_entry(i, &dev->all_adj_list.lower, list) {
- list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
- pr_debug("Interlinking %s with %s, non-neighbour\n",
- i->dev->name, j->dev->name);
- ret = __netdev_adjacent_dev_link(i->dev, j->dev);
- if (ret)
- goto rollback_mesh;
- }
- }
-
- /* add dev to every upper_dev's upper device */
- list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
- pr_debug("linking %s's upper device %s with %s\n",
- upper_dev->name, i->dev->name, dev->name);
- ret = __netdev_adjacent_dev_link(dev, i->dev);
- if (ret)
- goto rollback_upper_mesh;
- }
-
- /* add upper_dev to every dev's lower device */
- list_for_each_entry(i, &dev->all_adj_list.lower, list) {
- pr_debug("linking %s's lower device %s with %s\n", dev->name,
- i->dev->name, upper_dev->name);
- ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
- if (ret)
- goto rollback_lower_mesh;
- }
-
ret = call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, dev,
&changeupper_info.info);
ret = notifier_to_errno(ret);
if (ret)
- goto rollback_lower_mesh;
+ goto rollback;
return 0;
-rollback_lower_mesh:
- to_i = i;
- list_for_each_entry(i, &dev->all_adj_list.lower, list) {
- if (i == to_i)
- break;
- __netdev_adjacent_dev_unlink(i->dev, upper_dev, i->ref_nr);
- }
-
- i = NULL;
-
-rollback_upper_mesh:
- to_i = i;
- list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
- if (i == to_i)
- break;
- __netdev_adjacent_dev_unlink(dev, i->dev, i->ref_nr);
- }
-
- i = j = NULL;
-
-rollback_mesh:
- to_i = i;
- to_j = j;
- list_for_each_entry(i, &dev->all_adj_list.lower, list) {
- list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
- if (i == to_i && j == to_j)
- break;
- __netdev_adjacent_dev_unlink(i->dev, j->dev, i->ref_nr);
- }
- if (i == to_i)
- break;
- }
-
+rollback:
__netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
return ret;
@@ -5949,7 +5782,6 @@ void netdev_upper_dev_unlink(struct net_device *dev,
struct net_device *upper_dev)
{
struct netdev_notifier_changeupper_info changeupper_info;
- struct netdev_adjacent *i, *j;
ASSERT_RTNL();
changeupper_info.upper_dev = upper_dev;
@@ -5961,23 +5793,6 @@ void netdev_upper_dev_unlink(struct net_device *dev,
__netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
- /* Here is the tricky part. We must remove all dev's lower
- * devices from all upper_dev's upper devices and vice
- * versa, to maintain the graph relationship.
- */
- list_for_each_entry(i, &dev->all_adj_list.lower, list)
- list_for_each_entry(j, &upper_dev->all_adj_list.upper, list)
- __netdev_adjacent_dev_unlink(i->dev, j->dev, i->ref_nr);
-
- /* remove also the devices itself from lower/upper device
- * list
- */
- list_for_each_entry(i, &dev->all_adj_list.lower, list)
- __netdev_adjacent_dev_unlink(i->dev, upper_dev, i->ref_nr);
-
- list_for_each_entry(i, &upper_dev->all_adj_list.upper, list)
- __netdev_adjacent_dev_unlink(dev, i->dev, i->ref_nr);
-
call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, dev,
&changeupper_info.info);
}
@@ -7679,8 +7494,6 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
INIT_LIST_HEAD(&dev->link_watch_list);
INIT_LIST_HEAD(&dev->adj_list.upper);
INIT_LIST_HEAD(&dev->adj_list.lower);
- INIT_LIST_HEAD(&dev->all_adj_list.upper);
- INIT_LIST_HEAD(&dev->all_adj_list.lower);
INIT_LIST_HEAD(&dev->ptype_all);
INIT_LIST_HEAD(&dev->ptype_specific);
#ifdef CONFIG_NET_SCHED
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 08/11] rocker: Flip to the new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Convert rocker to the new dev walk API. This is just a code conversion;
no functional change is intended.
v2
- removed typecast of data
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
drivers/net/ethernet/rocker/rocker_main.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 5424fb341613..5deb25f26e5f 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2839,20 +2839,37 @@ static bool rocker_port_dev_check_under(const struct net_device *dev,
return true;
}
+struct rocker_walk_data {
+ struct rocker *rocker;
+ struct rocker_port *port;
+};
+
+static int rocker_lower_dev_walk(struct net_device *lower_dev, void *_data)
+{
+ struct rocker_walk_data *data = _data;
+ int ret = 0;
+
+ if (rocker_port_dev_check_under(lower_dev, data->rocker)) {
+ data->port = netdev_priv(lower_dev);
+ ret = 1;
+ }
+
+ return ret;
+}
+
struct rocker_port *rocker_port_dev_lower_find(struct net_device *dev,
struct rocker *rocker)
{
- struct net_device *lower_dev;
- struct list_head *iter;
+ struct rocker_walk_data data;
if (rocker_port_dev_check_under(dev, rocker))
return netdev_priv(dev);
- netdev_for_each_all_lower_dev(dev, lower_dev, iter) {
- if (rocker_port_dev_check_under(lower_dev, rocker))
- return netdev_priv(lower_dev);
- }
- return NULL;
+ data.rocker = rocker;
+ data.port = NULL;
+ netdev_walk_all_lower_dev(dev, rocker_lower_dev_walk, &data);
+
+ return data.port;
}
static int rocker_netdevice_event(struct notifier_block *unused,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 06/11] ixgbe: Flip to the new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Convert ixgbe users to new dev walk API. This is just a code conversion;
no functional change is intended.
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 132 ++++++++++++++++----------
1 file changed, 82 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 784b0b98ab2f..f380fda11eb6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5012,24 +5012,23 @@ static int ixgbe_fwd_ring_up(struct net_device *vdev,
return err;
}
-static void ixgbe_configure_dfwd(struct ixgbe_adapter *adapter)
+static int ixgbe_upper_dev_walk(struct net_device *upper, void *data)
{
- struct net_device *upper;
- struct list_head *iter;
- int err;
-
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
- if (netif_is_macvlan(upper)) {
- struct macvlan_dev *dfwd = netdev_priv(upper);
- struct ixgbe_fwd_adapter *vadapter = dfwd->fwd_priv;
+ if (netif_is_macvlan(upper)) {
+ struct macvlan_dev *dfwd = netdev_priv(upper);
+ struct ixgbe_fwd_adapter *vadapter = dfwd->fwd_priv;
- if (dfwd->fwd_priv) {
- err = ixgbe_fwd_ring_up(upper, vadapter);
- if (err)
- continue;
- }
- }
+ if (dfwd->fwd_priv)
+ ixgbe_fwd_ring_up(upper, vadapter);
}
+
+ return 0;
+}
+
+static void ixgbe_configure_dfwd(struct ixgbe_adapter *adapter)
+{
+ netdev_walk_all_upper_dev_rcu(adapter->netdev,
+ ixgbe_upper_dev_walk, NULL);
}
static void ixgbe_configure(struct ixgbe_adapter *adapter)
@@ -5448,12 +5447,25 @@ static void ixgbe_fdir_filter_exit(struct ixgbe_adapter *adapter)
spin_unlock(&adapter->fdir_perfect_lock);
}
+static int ixgbe_disable_macvlan(struct net_device *upper, void *data)
+{
+ if (netif_is_macvlan(upper)) {
+ struct macvlan_dev *vlan = netdev_priv(upper);
+
+ if (vlan->fwd_priv) {
+ netif_tx_stop_all_queues(upper);
+ netif_carrier_off(upper);
+ netif_tx_disable(upper);
+ }
+ }
+
+ return 0;
+}
+
void ixgbe_down(struct ixgbe_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
struct ixgbe_hw *hw = &adapter->hw;
- struct net_device *upper;
- struct list_head *iter;
int i;
/* signal that we are down to the interrupt handler */
@@ -5477,17 +5489,8 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
netif_tx_disable(netdev);
/* disable any upper devices */
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
- if (netif_is_macvlan(upper)) {
- struct macvlan_dev *vlan = netdev_priv(upper);
-
- if (vlan->fwd_priv) {
- netif_tx_stop_all_queues(upper);
- netif_carrier_off(upper);
- netif_tx_disable(upper);
- }
- }
- }
+ netdev_walk_all_upper_dev_rcu(adapter->netdev,
+ ixgbe_disable_macvlan, NULL);
ixgbe_irq_disable(adapter);
@@ -6728,6 +6731,18 @@ static void ixgbe_update_default_up(struct ixgbe_adapter *adapter)
#endif
}
+static int ixgbe_enable_macvlan(struct net_device *upper, void *data)
+{
+ if (netif_is_macvlan(upper)) {
+ struct macvlan_dev *vlan = netdev_priv(upper);
+
+ if (vlan->fwd_priv)
+ netif_tx_wake_all_queues(upper);
+ }
+
+ return 0;
+}
+
/**
* ixgbe_watchdog_link_is_up - update netif_carrier status and
* print link up message
@@ -6737,8 +6752,6 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
struct ixgbe_hw *hw = &adapter->hw;
- struct net_device *upper;
- struct list_head *iter;
u32 link_speed = adapter->link_speed;
const char *speed_str;
bool flow_rx, flow_tx;
@@ -6809,14 +6822,8 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
/* enable any upper devices */
rtnl_lock();
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
- if (netif_is_macvlan(upper)) {
- struct macvlan_dev *vlan = netdev_priv(upper);
-
- if (vlan->fwd_priv)
- netif_tx_wake_all_queues(upper);
- }
- }
+ netdev_walk_all_upper_dev_rcu(adapter->netdev,
+ ixgbe_enable_macvlan, NULL);
rtnl_unlock();
/* update the default user priority for VFs */
@@ -8350,12 +8357,38 @@ static int ixgbe_configure_clsu32_del_hnode(struct ixgbe_adapter *adapter,
}
#ifdef CONFIG_NET_CLS_ACT
+struct upper_walk_data {
+ struct ixgbe_adapter *adapter;
+ u64 action;
+ int ifindex;
+ u8 queue;
+};
+
+static int get_macvlan_queue(struct net_device *upper, void *_data)
+{
+ if (netif_is_macvlan(upper)) {
+ struct macvlan_dev *dfwd = netdev_priv(upper);
+ struct ixgbe_fwd_adapter *vadapter = dfwd->fwd_priv;
+ struct upper_walk_data *data = _data;
+ struct ixgbe_adapter *adapter = data->adapter;
+ int ifindex = data->ifindex;
+
+ if (vadapter && vadapter->netdev->ifindex == ifindex) {
+ data->queue = adapter->rx_ring[vadapter->rx_base_queue]->reg_idx;
+ data->action = data->queue;
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
static int handle_redirect_action(struct ixgbe_adapter *adapter, int ifindex,
u8 *queue, u64 *action)
{
unsigned int num_vfs = adapter->num_vfs, vf;
+ struct upper_walk_data data;
struct net_device *upper;
- struct list_head *iter;
/* redirect to a SRIOV VF */
for (vf = 0; vf < num_vfs; ++vf) {
@@ -8373,17 +8406,16 @@ static int handle_redirect_action(struct ixgbe_adapter *adapter, int ifindex,
}
/* redirect to a offloaded macvlan netdev */
- netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
- if (netif_is_macvlan(upper)) {
- struct macvlan_dev *dfwd = netdev_priv(upper);
- struct ixgbe_fwd_adapter *vadapter = dfwd->fwd_priv;
-
- if (vadapter && vadapter->netdev->ifindex == ifindex) {
- *queue = adapter->rx_ring[vadapter->rx_base_queue]->reg_idx;
- *action = *queue;
- return 0;
- }
- }
+ data.adapter = adapter;
+ data.ifindex = ifindex;
+ data.action = 0;
+ data.queue = 0;
+ if (netdev_walk_all_upper_dev_rcu(adapter->netdev,
+ get_macvlan_queue, &data)) {
+ *action = data.action;
+ *queue = data.queue;
+
+ return 0;
}
return -EINVAL;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 04/11] IB/core: Flip to the new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Convert rdma_is_upper_dev_rcu, handle_netdev_upper and
ipoib_get_net_dev_match_addr to the new upper device walk API.
This is just a code conversion; no functional change is intended.
v2
- removed typecast of data
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
drivers/infiniband/core/core_priv.h | 9 +------
drivers/infiniband/core/roce_gid_mgmt.c | 42 ++++++++++++++++++---------------
2 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 19d499dcab76..0c0bea091de8 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -127,14 +127,7 @@ void ib_cache_release_one(struct ib_device *device);
static inline bool rdma_is_upper_dev_rcu(struct net_device *dev,
struct net_device *upper)
{
- struct net_device *_upper = NULL;
- struct list_head *iter;
-
- netdev_for_each_all_upper_dev_rcu(dev, _upper, iter)
- if (_upper == upper)
- break;
-
- return _upper == upper;
+ return netdev_has_upper_dev_all_rcu(dev, upper);
}
int addr_init(void);
diff --git a/drivers/infiniband/core/roce_gid_mgmt.c b/drivers/infiniband/core/roce_gid_mgmt.c
index 06556c34606d..3a64a0881882 100644
--- a/drivers/infiniband/core/roce_gid_mgmt.c
+++ b/drivers/infiniband/core/roce_gid_mgmt.c
@@ -437,6 +437,28 @@ static void callback_for_addr_gid_device_scan(struct ib_device *device,
&parsed->gid_attr);
}
+struct upper_list {
+ struct list_head list;
+ struct net_device *upper;
+};
+
+static int netdev_upper_walk(struct net_device *upper, void *data)
+{
+ struct upper_list *entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ struct list_head *upper_list = data;
+
+ if (!entry) {
+ pr_info("roce_gid_mgmt: couldn't allocate entry to delete ndev\n");
+ return 0;
+ }
+
+ list_add_tail(&entry->list, upper_list);
+ dev_hold(upper);
+ entry->upper = upper;
+
+ return 0;
+}
+
static void handle_netdev_upper(struct ib_device *ib_dev, u8 port,
void *cookie,
void (*handle_netdev)(struct ib_device *ib_dev,
@@ -444,30 +466,12 @@ static void handle_netdev_upper(struct ib_device *ib_dev, u8 port,
struct net_device *ndev))
{
struct net_device *ndev = (struct net_device *)cookie;
- struct upper_list {
- struct list_head list;
- struct net_device *upper;
- };
- struct net_device *upper;
- struct list_head *iter;
struct upper_list *upper_iter;
struct upper_list *upper_temp;
LIST_HEAD(upper_list);
rcu_read_lock();
- netdev_for_each_all_upper_dev_rcu(ndev, upper, iter) {
- struct upper_list *entry = kmalloc(sizeof(*entry),
- GFP_ATOMIC);
-
- if (!entry) {
- pr_info("roce_gid_mgmt: couldn't allocate entry to delete ndev\n");
- continue;
- }
-
- list_add_tail(&entry->list, &upper_list);
- dev_hold(upper);
- entry->upper = upper;
- }
+ netdev_walk_all_upper_dev_rcu(ndev, netdev_upper_walk, &upper_list);
rcu_read_unlock();
handle_netdev(ib_dev, port, ndev);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 03/11] net: bonding: Flip to the new dev walk API
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Convert alb_send_learning_packets and bond_has_this_ip to use the new
netdev_walk_all_upper_dev_rcu API. In both cases this is just a code
conversion; no functional change is intended.
v2
- removed typecast of data and simplified bond_upper_dev_walk
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
drivers/net/bonding/bond_alb.c | 82 ++++++++++++++++++++++++++---------------
drivers/net/bonding/bond_main.c | 17 +++++----
2 files changed, 61 insertions(+), 38 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 551f0f8dead3..c80b023092dd 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -950,13 +950,61 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
dev_queue_xmit(skb);
}
+struct alb_walk_data {
+ struct bonding *bond;
+ struct slave *slave;
+ u8 *mac_addr;
+ bool strict_match;
+};
+
+static int alb_upper_dev_walk(struct net_device *upper, void *_data)
+{
+ struct alb_walk_data *data = _data;
+ bool strict_match = data->strict_match;
+ struct bonding *bond = data->bond;
+ struct slave *slave = data->slave;
+ u8 *mac_addr = data->mac_addr;
+ struct bond_vlan_tag *tags;
+
+ if (is_vlan_dev(upper) && vlan_get_encap_level(upper) == 0) {
+ if (strict_match &&
+ ether_addr_equal_64bits(mac_addr,
+ upper->dev_addr)) {
+ alb_send_lp_vid(slave, mac_addr,
+ vlan_dev_vlan_proto(upper),
+ vlan_dev_vlan_id(upper));
+ } else if (!strict_match) {
+ alb_send_lp_vid(slave, upper->dev_addr,
+ vlan_dev_vlan_proto(upper),
+ vlan_dev_vlan_id(upper));
+ }
+ }
+
+ /* If this is a macvlan device, then only send updates
+ * when strict_match is turned off.
+ */
+ if (netif_is_macvlan(upper) && !strict_match) {
+ tags = bond_verify_device_path(bond->dev, upper, 0);
+ if (IS_ERR_OR_NULL(tags))
+ BUG();
+ alb_send_lp_vid(slave, upper->dev_addr,
+ tags[0].vlan_proto, tags[0].vlan_id);
+ kfree(tags);
+ }
+
+ return 0;
+}
+
static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[],
bool strict_match)
{
struct bonding *bond = bond_get_bond_by_slave(slave);
- struct net_device *upper;
- struct list_head *iter;
- struct bond_vlan_tag *tags;
+ struct alb_walk_data data = {
+ .strict_match = strict_match,
+ .mac_addr = mac_addr,
+ .slave = slave,
+ .bond = bond,
+ };
/* send untagged */
alb_send_lp_vid(slave, mac_addr, 0, 0);
@@ -965,33 +1013,7 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[],
* for that device.
*/
rcu_read_lock();
- netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
- if (is_vlan_dev(upper) && vlan_get_encap_level(upper) == 0) {
- if (strict_match &&
- ether_addr_equal_64bits(mac_addr,
- upper->dev_addr)) {
- alb_send_lp_vid(slave, mac_addr,
- vlan_dev_vlan_proto(upper),
- vlan_dev_vlan_id(upper));
- } else if (!strict_match) {
- alb_send_lp_vid(slave, upper->dev_addr,
- vlan_dev_vlan_proto(upper),
- vlan_dev_vlan_id(upper));
- }
- }
-
- /* If this is a macvlan device, then only send updates
- * when strict_match is turned off.
- */
- if (netif_is_macvlan(upper) && !strict_match) {
- tags = bond_verify_device_path(bond->dev, upper, 0);
- if (IS_ERR_OR_NULL(tags))
- BUG();
- alb_send_lp_vid(slave, upper->dev_addr,
- tags[0].vlan_proto, tags[0].vlan_id);
- kfree(tags);
- }
- }
+ netdev_walk_all_upper_dev_rcu(bond->dev, alb_upper_dev_walk, &data);
rcu_read_unlock();
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5fa36ebc0640..c9944d86d045 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2270,22 +2270,23 @@ static void bond_mii_monitor(struct work_struct *work)
}
}
+static int bond_upper_dev_walk(struct net_device *upper, void *data)
+{
+ __be32 ip = *((__be32 *)data);
+
+ return ip == bond_confirm_addr(upper, 0, ip);
+}
+
static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
{
- struct net_device *upper;
- struct list_head *iter;
bool ret = false;
if (ip == bond_confirm_addr(bond->dev, 0, ip))
return true;
rcu_read_lock();
- netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
- if (ip == bond_confirm_addr(upper, 0, ip)) {
- ret = true;
- break;
- }
- }
+ if (netdev_walk_all_upper_dev_rcu(bond->dev, bond_upper_dev_walk, &ip))
+ ret = true;
rcu_read_unlock();
return ret;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 01/11] net: Remove refnr arg when inserting link adjacencies
From: David Ahern @ 2016-10-18 2:15 UTC (permalink / raw)
To: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A, David Ahern
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Commit 93409033ae65 ("net: Add netdev all_adj_list refcnt propagation to
fix panic") propagated the refnr to insert and remove functions tracking
the netdev adjacency graph. However, for the insert path the refnr can
only be 1. Accordingly, remove the refnr argument to make that clear.
ie., the refnr arg in 93409033ae65 was only needed for the remove path.
Signed-off-by: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
---
net/core/dev.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 352e98129601..f67fd16615bb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5453,7 +5453,6 @@ static inline bool netdev_adjacent_is_neigh_list(struct net_device *dev,
static int __netdev_adjacent_dev_insert(struct net_device *dev,
struct net_device *adj_dev,
- u16 ref_nr,
struct list_head *dev_list,
void *private, bool master)
{
@@ -5463,7 +5462,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj = __netdev_find_adj(adj_dev, dev_list);
if (adj) {
- adj->ref_nr += ref_nr;
+ adj->ref_nr += 1;
return 0;
}
@@ -5473,7 +5472,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj->dev = adj_dev;
adj->master = master;
- adj->ref_nr = ref_nr;
+ adj->ref_nr = 1;
adj->private = private;
dev_hold(adj_dev);
@@ -5547,22 +5546,21 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
static int __netdev_adjacent_dev_link_lists(struct net_device *dev,
struct net_device *upper_dev,
- u16 ref_nr,
struct list_head *up_list,
struct list_head *down_list,
void *private, bool master)
{
int ret;
- ret = __netdev_adjacent_dev_insert(dev, upper_dev, ref_nr, up_list,
+ ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list,
private, master);
if (ret)
return ret;
- ret = __netdev_adjacent_dev_insert(upper_dev, dev, ref_nr, down_list,
+ ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list,
private, false);
if (ret) {
- __netdev_adjacent_dev_remove(dev, upper_dev, ref_nr, up_list);
+ __netdev_adjacent_dev_remove(dev, upper_dev, 1, up_list);
return ret;
}
@@ -5570,10 +5568,9 @@ static int __netdev_adjacent_dev_link_lists(struct net_device *dev,
}
static int __netdev_adjacent_dev_link(struct net_device *dev,
- struct net_device *upper_dev,
- u16 ref_nr)
+ struct net_device *upper_dev)
{
- return __netdev_adjacent_dev_link_lists(dev, upper_dev, ref_nr,
+ return __netdev_adjacent_dev_link_lists(dev, upper_dev,
&dev->all_adj_list.upper,
&upper_dev->all_adj_list.lower,
NULL, false);
@@ -5602,12 +5599,12 @@ static int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
struct net_device *upper_dev,
void *private, bool master)
{
- int ret = __netdev_adjacent_dev_link(dev, upper_dev, 1);
+ int ret = __netdev_adjacent_dev_link(dev, upper_dev);
if (ret)
return ret;
- ret = __netdev_adjacent_dev_link_lists(dev, upper_dev, 1,
+ ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
&dev->adj_list.upper,
&upper_dev->adj_list.lower,
private, master);
@@ -5676,7 +5673,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
pr_debug("Interlinking %s with %s, non-neighbour\n",
i->dev->name, j->dev->name);
- ret = __netdev_adjacent_dev_link(i->dev, j->dev, i->ref_nr);
+ ret = __netdev_adjacent_dev_link(i->dev, j->dev);
if (ret)
goto rollback_mesh;
}
@@ -5686,7 +5683,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
pr_debug("linking %s's upper device %s with %s\n",
upper_dev->name, i->dev->name, dev->name);
- ret = __netdev_adjacent_dev_link(dev, i->dev, i->ref_nr);
+ ret = __netdev_adjacent_dev_link(dev, i->dev);
if (ret)
goto rollback_upper_mesh;
}
@@ -5695,7 +5692,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
list_for_each_entry(i, &dev->all_adj_list.lower, list) {
pr_debug("linking %s's lower device %s with %s\n", dev->name,
i->dev->name, upper_dev->name);
- ret = __netdev_adjacent_dev_link(i->dev, upper_dev, i->ref_nr);
+ ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
if (ret)
goto rollback_lower_mesh;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net-next 1/1] net: vlan: Use sizeof instead of literal number
From: Gao Feng @ 2016-10-18 1:49 UTC (permalink / raw)
To: David Miller; +Cc: Patrick McHardy, Linux Kernel Network Developers
In-Reply-To: <20161017.213658.349412154638384827.davem@davemloft.net>
Hi David,
On Tue, Oct 18, 2016 at 9:36 AM, David Miller <davem@davemloft.net> wrote:
>
> It never makes sense to send the same patch for both net and net-next.
>
> If it's a bug fix, it goes to 'net'. And it will be eventually
> be naturally merged into 'net-next'.
>
> Otherwise, if it's a new feature, cleanup, or optimization it goes to
> 'net-next'.
Because I forget add the "net-next" in the title of first patch, so I
send the second patch with right title.
And I have replied the first patch and said the reason.
Regards
Feng
^ permalink raw reply
* Re: [PATCH net-next 1/1] net: vlan: Use sizeof instead of literal number
From: David Miller @ 2016-10-18 1:36 UTC (permalink / raw)
To: fgao; +Cc: kaber, netdev, gfree.wind
In-Reply-To: <1476752270-12590-1-git-send-email-fgao@ikuai8.com>
It never makes sense to send the same patch for both net and net-next.
If it's a bug fix, it goes to 'net'. And it will be eventually
be naturally merged into 'net-next'.
Otherwise, if it's a new feature, cleanup, or optimization it goes to
'net-next'.
^ permalink raw reply
* Re: [PATCH 1/1] net: vlan: Use sizeof instead of literal number
From: Feng Gao @ 2016-10-18 0:59 UTC (permalink / raw)
To: Patrick McHardy, David S. Miller, Linux Kernel Network Developers
Cc: Feng Gao, Gao Feng
In-Reply-To: <1476751442-12112-1-git-send-email-fgao@ikuai8.com>
On Tue, Oct 18, 2016 at 8:44 AM, <fgao@ikuai8.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> Use sizeof variable instead of literal number to enhance the readability.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> net/8021q/vlan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index 8de138d..5a3903b 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -515,8 +515,8 @@ static int vlan_ioctl_handler(struct net *net, void __user *arg)
> return -EFAULT;
>
> /* Null terminate this sucker, just in case. */
> - args.device1[23] = 0;
> - args.u.device2[23] = 0;
> + args.device1[sizeof(args.device1) - 1] = 0;
> + args.u.device2[sizeof(args.u.device2) - 1] = 0;
>
> rtnl_lock();
>
> --
> 1.9.1
>
>
Sorry, I forget add the "net-next" in the title.
Now I have sent another new patch, please ignore this conversation.
Regards
Feng
^ permalink raw reply
* [PATCH net-next 1/1] net: vlan: Use sizeof instead of literal number
From: fgao @ 2016-10-18 0:57 UTC (permalink / raw)
To: kaber, davem, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
Use sizeof variable instead of literal number to enhance the readability.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/8021q/vlan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8de138d..5a3903b 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -515,8 +515,8 @@ static int vlan_ioctl_handler(struct net *net, void __user *arg)
return -EFAULT;
/* Null terminate this sucker, just in case. */
- args.device1[23] = 0;
- args.u.device2[23] = 0;
+ args.device1[sizeof(args.device1) - 1] = 0;
+ args.u.device2[sizeof(args.u.device2) - 1] = 0;
rtnl_lock();
--
1.9.1
^ permalink raw reply related
* [PATCH 1/1] net: vlan: Use sizeof instead of literal number
From: fgao @ 2016-10-18 0:44 UTC (permalink / raw)
To: kaber, davem, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
Use sizeof variable instead of literal number to enhance the readability.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/8021q/vlan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8de138d..5a3903b 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -515,8 +515,8 @@ static int vlan_ioctl_handler(struct net *net, void __user *arg)
return -EFAULT;
/* Null terminate this sucker, just in case. */
- args.device1[23] = 0;
- args.u.device2[23] = 0;
+ args.device1[sizeof(args.device1) - 1] = 0;
+ args.u.device2[sizeof(args.u.device2) - 1] = 0;
rtnl_lock();
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
From: David Ahern @ 2016-10-18 0:42 UTC (permalink / raw)
To: Stephen Hemminger
Cc: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A
In-Reply-To: <20161017052121.2322279d@xeon-e3>
On 10/17/16 6:21 AM, Stephen Hemminger wrote:
>
> No if/else needed. No cast of void * ptr need. Use const if possible?
>
so much of the stack does not use const and trying to add it for this API does not work -- the upper or lower device is passed to the callbacks and those callbacks invoke other apis. e.g., the bond patch calls vlan_get_encap_level, bond_verify_device_path and bond_confirm_addr and none of those accept a const dev.
v3 coming up with the more succinct versions, but const is not possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch net-next RFC 4/6] Introduce sample tc action
From: Roopa Prabhu @ 2016-10-18 0:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, netdev, davem, yotamg, idosch, eladr, nogahf,
ogerlitz, geert+renesas, stephen, xiyou.wangcong, linux,
Shrijeet Mukherjee
In-Reply-To: <81e49d64-e0bc-2f43-7a9c-daefb12a4b8a@mojatatu.com>
On 10/17/16, 3:10 AM, Jamal Hadi Salim wrote:
>
> Some comments:
> IIUC, the main struggle seems to be whether the redirect to dummy0
> is useful or not? i.e instead of just letting the packets go up the
> stack on eth1?
yep, correct...given existing workflow for the non-offloaded case is
to receive sample packets via bpf filter on socket or
use netlink as a sample delivery mechanism (NFLOG eg)
> It seems like sflowd needs to read off eth1 via packet socket?
> To be backward compatible - supporting that approach seems sensible.
>
> Note:
> There is a clear efficiency benefit of both using IFE encoding and
> redirecting to dummy0.
> 1) Redirecting to dummy0 implies you dont need to exercise a bpf
> filter around every packet that comes off eth1.
> I understand there are probably not millions of pps for this case;
> but in a non-offloaded cases it could be millions pps.
> And in case of sampling over many ethx devices, you can redirect
> samples from many other ethx devices.
> So making dummy0 the sflow device is a win.
> 2) Encaping an IFE header implies a much more efficient bpf filter
> (IFE ethertype is an excellent discriminator for bpf).
>
> Additional benefit is as mentioned before - redirecting to a device
> means you can send it remotely over ethernet to a more powerful
> machine without having to cross kernel-userspace. Redirecting instead
> of mirroring to tuntap is also an interesting option.
sure, this seems like a good option to have.
generally you have one instance of the sampling agent on a hyper visor or switch.
But, if you have use-cases where monitoring agents run external, sure.
would have preferred if it was optional or an addon and not the default.
Regarding the device, yeah, agree there are pros and cons.
An additional device just to sample packets seems like an overkill.
But, if there is no other other option, and there are benefits to it, no objections.
Hopefully we can add another option on the existing api to skip the device in the future.
>
>
> On 16-10-15 12:34 PM, Roopa Prabhu wrote:
>> On 10/12/16, 5:41 AM, Jiri Pirko wrote:
>>> From: Yotam Gigi <yotam.gi@gmail.com>
>
>
>>> +
>>> +struct sample_packet_metadata {
>>> + int sample_size;
>>> + int orig_size;
>>> + int ifindex;
>>> +};
>>> +
>> This metadata does not look extensible.. can it be made to ?
>>
>
> Sure it can...
>
>> With sflow in context, you need a pair of ifindex numbers to encode ingress and egress ports.
>
> What is the use case for both?
I have heard that most monitoring tools have moved to ingress only sampling because of operational
complexity (use case is sflow). I think hardware also supports ingress and egress only sampling.
better to have an option to reflect that in the api.
>> Ideally you would also include a sequence number and a count of the total number of packets
> > that were candidates for sampling.
>
> Sequence number may make sense (they will help show a gap if something
> gets dropped). But i am not sure about the stats consuming such space.
> Stats are something that can be queried (tc stats should have a record
> of how many bytes/packets )
sure, thats fine.
>
>> The OVS implementation is a good example, the metadata includes all the actions applied
>> to the packet in the kernel data path.
>>
>
> Again not sure what the use case would be (and why waste such space
> especially when you are sending over the wire with such details).
All this is being used currently.., But, this can be other api's sflow uses
for monitoring.
http://openvswitch.org/support/ovscon2014/17/1400-ovs-sflow.pdf
Does not have to be part of the main/basic sampling api...
it was just an example.
>
>>> + rcu_read_lock();
>>> + retval = READ_ONCE(s->tcf_action);
>>> +
>>> + if (++s->packet_counter % s->rate == 0) {
>>
>> The sampling function isn’t random
>>
>> if (++s->packet_counter % s->rate == 0) {
>>
>> This is unsuitable for sFlow, which is specific about the random sampling function required.
>> BPF, OVS, and the
>> ULOG statistics module include efficient kernel based random sampling functions that could be used instead.
>>
>
> If i understood correctly, the above is a fallback sampling algorithm.
> In the case of the spectrum it already does the sampling in the ASIC
> so there is no need to repeat it in software.
> Agreed that in that case the sampling approach is not sufficiently
> random.
yes. and since the same sampling api will be used for offloaded and non-offloaded case,
the sampling algo here for the non-offloaded case...can do better .. atleast match the existing
api efficiency. We would want people to use the same api for the offload and non-offloaded case.
thanks,
Roopa
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH V3 RFC 2/2] ixgbe: ixgbe_atr() compute l4_proto only if non-paged data has network/transport headers
From: Jeff Kirsher @ 2016-10-17 22:48 UTC (permalink / raw)
To: Sowmini Varadhan
Cc: Alexander Duyck, Duyck, Alexander H, Netdev, intel-wired-lan
In-Reply-To: <20161017224702.GA17588@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 500 bytes --]
On Mon, 2016-10-17 at 18:47 -0400, Sowmini Varadhan wrote:
> On (10/17/16 15:37), Jeff Kirsher wrote:
> > > Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > Sowmini, can you re-submit this to intel-wired-lan but without the RFC
> in
> > the title?
>
> V4 resubmitted.. I think I just inadvertently forgot to add Alex as the
> reviewed-by.. could you please fix that (or I can resubmit v5 if needed).
No need to resubmit, I can make sure Alex's reviewed-by gets added.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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