* Re: [PATCH 1/2] net: mv643xx_eth: use phydev from struct net_device
From: David Miller @ 2016-10-12 5:40 UTC (permalink / raw)
To: tremyfr; +Cc: sebastian.hesselbarth, netdev, linux-kernel
In-Reply-To: <1475402809-11593-1-git-send-email-tremyfr@gmail.com>
From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 2 Oct 2016 12:06:48 +0200
> The private structure contain a pointer to phydev, but the structure
> net_device already contain such pointer. So we can remove the pointer
> phydev in the private structure, and update the driver to use the
> one contained in struct net_device.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Chris Rorvick @ 2016-10-12 4:32 UTC (permalink / raw)
To: Paul Bolle
Cc: Luca Coelho, Intel Linux Wireless, Emmanuel Grumbach,
Johannes Berg, Kalle Valo, Oren Givon, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1476180668.17022.21.camel@tiscali.nl>
On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> For what it's worth, on my machine I have twenty (!) SPLX entries, all
> reading:
> Name (SPLX, Package (0x04)
> {
> Zero,
> Package (0x03)
> {
> 0x80000000,
> 0x80000000,
> 0x80000000
> },
>
> Package (0x03)
> {
> 0x80000000,
> 0x80000000,
> 0x80000000
> },
>
> Package (0x03)
> {
> 0x80000000,
> 0x80000000,
> 0x80000000
> }
> })
I actually see exactly the same on my Dell XPS 13 (9350) when I use
acpidump, etc. I typed the entry I included in the commit log by hand
based on what the driver gets back from the SPLC method (I added a
function to dump the returned object.)
Chris
^ permalink raw reply
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Yuchung Cheng @ 2016-10-12 3:56 UTC (permalink / raw)
To: Lawrence Brakmo; +Cc: netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <CAK6E8=c8yHaEsfqRW9O1z7Dj21iU96QqY6nkSCtUpTbKOOc8wQ@mail.gmail.com>
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-C8ltkgyx1u_
>>>g&m=Q4nONH7kQ5AvQguw9UxpcHd79jfdDdrXj1YSJs7Ezhk&s=MA4fWBLMTGgRS0eGvBjxf7BJ
>>>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
* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Saeed Mahameed @ 2016-10-12 2:03 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Tom Herbert, David Laight, davem@davemloft.net,
netdev@vger.kernel.org, saeedm@mellanox.com, kernel-team@fb.com
In-Reply-To: <20161011194000.GO9282@leon.nu>
On Wed, Oct 12, 2016 at 4:40 AM, Leon Romanovsky <leon@kernel.org> wrote:
> On Tue, Oct 11, 2016 at 08:46:45AM -0700, Tom Herbert wrote:
>> On Tue, Oct 11, 2016 at 4:57 AM, Saeed Mahameed
>> <saeedm@dev.mellanox.co.il> wrote:
>> > On Tue, Oct 11, 2016 at 7:50 PM, David Laight <David.Laight@aculab.com> wrote:
>> >> From: Tom Herbert
>> >>> Sent: 11 October 2016 05:22
>> >> ...
>> >>> Fix is to create MLX5_SET64_VCHK that takes an additional argument
>> >>> that is a constant. There are two callers of MLX5_SET64 that are
>> >>> trying to get a variable offset, change those to call MLX5_SET64_VCHK
>> >>> passing pas[0] as the argument to use in the offset check.
>> >>
>> >> I think I'd separate the array index instead.
>> >> Something like:
>> >>
>> >> #define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
>> >> BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
>> >> __MLX5_SET64(typ, p, fld[ndx], v); \
>> >> } while (0)
>> >>
>> >> David
>> >
>> > Yes, I think this looks more natural, but instead MLX5_SET64_INDEXED,
>> > I prefer to have 2 macros
>> > MLX5_SET64(typ, p, fld, v) and MLX5_ARRAY_SET64(typ, p, fld, idx, v).
>> >
>> > Tom, do you want me to fix it ?
>> >
>> Please do.
>
> Saeed,
>
> Do you success to send this patch before -rc1 is released? So Linus's
> -rc1 will be clean from such build error.
>
Just submitted the patch, it seems that i have issues with my other
Mailer, sometimes e-mails take a while to appear in the mailing list.
I Hope the patch will arrive on time for Dave to pick it it up.
Thanks,
-Saeed.
^ permalink raw reply
* [PATCH V2 net-next] net/mlx5: Add MLX5_ARRAY_SET64 to fix BUILD_BUG_ON
From: Saeed Mahameed @ 2016-10-12 1:57 UTC (permalink / raw)
To: David S. Miller, netdev, Tom Herbert
Cc: Leon Romanovsky, David Laight, kernel-team, Saeed Mahameed
From: Tom Herbert <tom@herbertland.com>
I am hitting this in mlx5:
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function
reclaim_pages_cmd.clone.0:
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:346: error: call
to __compiletime_assert_346 declared with attribute error:
BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_out, pas[i]) % 64
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function give_pages:
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:291: error: call
to __compiletime_assert_291 declared with attribute error:
BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_in, pas[i]) % 64
Problem is that this is doing a BUILD_BUG_ON on a non-constant
expression because of trying to take offset of pas[i] in the
structure.
Fix is to create MLX5_ARRAY_SET64 that takes an additional argument
that is the field index to separate between BUILD_BUG_ON on the array
constant field and the indexed field to assign the value to.
There are two callers of MLX5_SET64 that are trying to get a variable
offset, change those to call MLX5_ARRAY_SET64 passing 'pas' and 'i'
as the arguments to use in the offset check and the indexed value
assignment.
Fixes: a533ed5e179cd ("net/mlx5: Pages management commands via mlx5 ifc")
Signed-off-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
Hi Dave,
I hope this version of this patch will make it to -rc1, I made
some changes to the original version Tom submitted. Following David Laight
suggestion to separate the array index from the constant array field to have
a more natural API.
Thanks,
Saeed.
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 4 ++--
include/linux/mlx5/device.h | 13 +++++++++++--
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
index d458515..cc4fd61 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
@@ -287,7 +287,7 @@ retry:
goto retry;
}
- MLX5_SET64(manage_pages_in, in, pas[i], addr);
+ MLX5_ARRAY_SET64(manage_pages_in, in, pas, i, addr);
}
MLX5_SET(manage_pages_in, in, opcode, MLX5_CMD_OP_MANAGE_PAGES);
@@ -344,7 +344,7 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
if (fwp->func_id != func_id)
continue;
- MLX5_SET64(manage_pages_out, out, pas[i], fwp->addr);
+ MLX5_ARRAY_SET64(manage_pages_out, out, pas, i, fwp->addr);
i++;
}
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 77c1417..5827614 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -92,12 +92,21 @@ __mlx5_mask(typ, fld))
___t; \
})
-#define MLX5_SET64(typ, p, fld, v) do { \
+#define __MLX5_SET64(typ, p, fld, v) do { \
BUILD_BUG_ON(__mlx5_bit_sz(typ, fld) != 64); \
- BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
*((__be64 *)(p) + __mlx5_64_off(typ, fld)) = cpu_to_be64(v); \
} while (0)
+#define MLX5_SET64(typ, p, fld, v) do { \
+ BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
+ __MLX5_SET64(typ, p, fld, v); \
+} while (0)
+
+#define MLX5_ARRAY_SET64(typ, p, fld, idx, v) do { \
+ BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
+ __MLX5_SET64(typ, p, fld[idx], v); \
+} while (0)
+
#define MLX5_GET64(typ, p, fld) be64_to_cpu(*((__be64 *)(p) + __mlx5_64_off(typ, fld)))
#define MLX5_GET64_PR(typ, p, fld) ({ \
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Yuchung Cheng @ 2016-10-12 1:01 UTC (permalink / raw)
To: Lawrence Brakmo; +Cc: netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <D42294A7.1FB0A%brakmo@fb.com>
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-C8ltkgyx1u_
>>g&m=Q4nONH7kQ5AvQguw9UxpcHd79jfdDdrXj1YSJs7Ezhk&s=MA4fWBLMTGgRS0eGvBjxf7BJ
>>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?
>
>>
>>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 v2] bridge: add address and vlan to fdb warning messages
From: Roopa Prabhu @ 2016-10-11 23:12 UTC (permalink / raw)
To: davem, stephen; +Cc: nikolay, netdev
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds vlan and address to warning messages printed
in the bridge fdb code for debuggability.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
v2 - address comments from stephen
- use %u format specifier for vlan
- move print string to a single line
net/bridge/br_fdb.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6b43c8c..e4a4176 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -535,9 +535,8 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
*/
if (fdb->is_local)
return 0;
- br_warn(br, "adding interface %s with same address "
- "as a received packet\n",
- source ? source->dev->name : br->dev->name);
+ br_warn(br, "adding interface %s with same address as a received packet (addr:%pM, vlan:%u)\n",
+ source ? source->dev->name : br->dev->name, addr, vid);
fdb_delete(br, fdb);
}
@@ -583,9 +582,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
/* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
if (net_ratelimit())
- br_warn(br, "received packet on %s with "
- "own address as source address\n",
- source->dev->name);
+ br_warn(br, "received packet on %s with own address as source address (addr:%pM, vlan:%u)\n",
+ source->dev->name, addr, vid);
} else {
/* fastpath: update of existing entry */
if (unlikely(source != fdb->dst)) {
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] bridge: add address and vlan to fdb warning messages
From: Roopa Prabhu @ 2016-10-11 22:30 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, nikolay, netdev
In-Reply-To: <20161011151720.33f67320@xeon-e3>
On 10/11/16, 3:17 PM, Stephen Hemminger wrote:
> On Tue, 11 Oct 2016 14:33:51 -0700
> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds vlan and address to warning messages printed
>> in the bridge fdb code for debuggability.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> net/bridge/br_fdb.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6b43c8c..1257a88 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -536,8 +536,8 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> if (fdb->is_local)
>> return 0;
>> br_warn(br, "adding interface %s with same address "
>> - "as a received packet\n",
>> - source ? source->dev->name : br->dev->name);
>> + "as a received packet (addr:%pM, vlan:%d)\n",
>> + source ? source->dev->name : br->dev->name, addr, vid);
>> fdb_delete(br, fdb);
>> }
>>
>> @@ -583,9 +583,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> /* attempt to update an entry for a local interface */
>> if (unlikely(fdb->is_local)) {
>> if (net_ratelimit())
>> - br_warn(br, "received packet on %s with "
>> - "own address as source address\n",
>> - source->dev->name);
>> + br_warn(br, "received packet on %s with own "
>> + "address as source address "
>> + "(addr:%pM, vlan:%d)\n",
>> + source->dev->name, addr, vid);
>> } else {
> Isn't vlan unsigned here so print with %u
you are right. will fix it
> Also it would be good to make string format on one line to allow for easy search of source.
> br_warn(br,
> "received packet on %s with own address %pM vlan %u",
> source->dev->name, addr, vid);
ack, was debating abt this.
v2 coming..thanks
^ permalink raw reply
* Re: [PATCH net-next] bridge: add address and vlan to fdb warning messages
From: Stephen Hemminger @ 2016-10-11 22:17 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, nikolay, netdev
In-Reply-To: <1476221632-18488-1-git-send-email-roopa@cumulusnetworks.com>
On Tue, 11 Oct 2016 14:33:51 -0700
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds vlan and address to warning messages printed
> in the bridge fdb code for debuggability.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> net/bridge/br_fdb.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6b43c8c..1257a88 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -536,8 +536,8 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
> if (fdb->is_local)
> return 0;
> br_warn(br, "adding interface %s with same address "
> - "as a received packet\n",
> - source ? source->dev->name : br->dev->name);
> + "as a received packet (addr:%pM, vlan:%d)\n",
> + source ? source->dev->name : br->dev->name, addr, vid);
> fdb_delete(br, fdb);
> }
>
> @@ -583,9 +583,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> /* attempt to update an entry for a local interface */
> if (unlikely(fdb->is_local)) {
> if (net_ratelimit())
> - br_warn(br, "received packet on %s with "
> - "own address as source address\n",
> - source->dev->name);
> + br_warn(br, "received packet on %s with own "
> + "address as source address "
> + "(addr:%pM, vlan:%d)\n",
> + source->dev->name, addr, vid);
> } else {
Isn't vlan unsigned here so print with %u
Also it would be good to make string format on one line to allow for easy search of source.
br_warn(br,
"received packet on %s with own address %pM vlan %u",
source->dev->name, addr, vid);
^ permalink raw reply
* 4989658971444 netdev
From: kkmomofr @ 2016-10-11 22:16 UTC (permalink / raw)
To: netdev
[-- Attachment #1: EMAIL_3992939235268_netdev.zip --]
[-- Type: application/zip, Size: 4178 bytes --]
^ permalink raw reply
* Re: saddr based blackhole/unreachable route
From: Bjørnar Ness @ 2016-10-11 22:13 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAJO99Tn7oaGEhRKEBR83K_+JSJUaFXEqvMnbM3vOLVxDqF9wtw@mail.gmail.com>
2016-10-11 20:39 GMT+02:00 Bjørnar Ness <bjornar.ness@gmail.com>:
> Hello, netdev
>
> In a typical setup (eth0=internet, eth1=lan) i populate routing table
> 100 with saddrs I want
> dropped, and: "ip ru a pref 100 lookup table 100"
>
> What I would expect to see is packets with a saddr in table 100 coming
> in eth0 will go out eth1,
> with replies beeing dropped, but I do not see the packets going out eth1 at all.
>
> Have tried searching and following the fib codepath, but have still
> not managed to understand what is really going on here.
Answering my own question, I guess its rp_filter kicking in here. and
it also explains
why I dont get icmp unreachable
Is there a better way to do source based rtbh?
--
Bj(/)rnar
^ permalink raw reply
* [PATCH] Temporary patch for arpd
From: Pascal @ 2016-10-11 21:50 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 919 bytes --]
Hello. I found wonderful bug in arpd daemon of iproute2 package.
Somehow arpd is absolute unworkable if run program with -f flag. On my
amd64 server i got "Segmentation fault" regardless -f mac-list.txt file
content.
The source of misc/arpd.c is not hard and i found that cause of
this bug is commit dd50247dba85255538d659551305b4bb75bcae62. I'm not
c++ developer, but i suppose segfault occured because argument of
dbase->put() has non-initialized dbdat.data argument.
Also arpd.c has strange condition "if (do_load || do_list)" that not
allows to run program with -f argument.
I did pull out the hexstring_a2n function from utils.c of previous commit
aeb199d5ce86c6c72decaac333cad5a7d7b38b3a and used it to populate
dbdat.data value after which program works fine.
I hurriedly make the patch that makes program alive. Please inspect
this problem, fix this bug and test program with -f key.
PS: sorry for my english =)
[-- Attachment #2: 0001-Temporary-patch-for-arpd.patch --]
[-- Type: application/octet-stream, Size: 1694 bytes --]
From 433147a7303c418845bdb5668910caababf79453 Mon Sep 17 00:00:00 2001
From: Pascal <pascal@pascalhp.net>
Date: Wed, 12 Oct 2016 03:13:53 +0800
Subject: [PATCH] Temporary patch for arpd
---
misc/arpd.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/misc/arpd.c b/misc/arpd.c
index bfab445..567df5b 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -584,6 +584,43 @@ static void send_stats(void)
do_stats = 0;
}
+__u8* hexstring_a2n_old(const char *str, __u8 *buf, int blen)
+{
+ int cnt = 0;
+
+ for (;;) {
+ unsigned acc;
+ char ch;
+
+ acc = 0;
+
+ while ((ch = *str) != ':' && ch != 0) {
+ if (ch >= '0' && ch <= '9')
+ ch -= '0';
+ else if (ch >= 'a' && ch <= 'f')
+ ch -= 'a'-10;
+ else if (ch >= 'A' && ch <= 'F')
+ ch -= 'A'-10;
+ else
+ return NULL;
+ acc = (acc<<4) + ch;
+ str++;
+ }
+
+ if (acc > 255)
+ return NULL;
+ if (cnt < blen) {
+ buf[cnt] = acc;
+ cnt++;
+ }
+ if (ch == 0)
+ break;
+ ++str;
+ }
+ if (cnt < blen)
+ memset(buf+cnt, 0, blen-cnt);
+ return buf;
+}
int main(int argc, char **argv)
{
@@ -715,8 +752,11 @@ int main(int argc, char **argv)
goto do_abort;
}
- if (ll_addr_a2n((char *) b1, 6, macbuf) != 6)
+ dbdat.data = hexstring_a2n_old(macbuf, b1, 6);
+ if (dbdat.data == NULL) {
+ fprintf(stderr, "Invalid MAC address: \"%s\"\n", macbuf);
goto do_abort;
+ }
dbdat.size = 6;
if (dbase->put(dbase, &dbkey, &dbdat, 0)) {
@@ -754,7 +794,7 @@ int main(int argc, char **argv)
}
}
- if (do_load || do_list)
+ if (do_list)
goto out;
pset[0].fd = socket(PF_PACKET, SOCK_DGRAM, 0);
--
2.7.0.windows.2
^ permalink raw reply related
* [PATCH net-next] bridge: add address and vlan to fdb warning messages
From: Roopa Prabhu @ 2016-10-11 21:33 UTC (permalink / raw)
To: davem, stephen; +Cc: nikolay, netdev
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds vlan and address to warning messages printed
in the bridge fdb code for debuggability.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
net/bridge/br_fdb.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6b43c8c..1257a88 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -536,8 +536,8 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
if (fdb->is_local)
return 0;
br_warn(br, "adding interface %s with same address "
- "as a received packet\n",
- source ? source->dev->name : br->dev->name);
+ "as a received packet (addr:%pM, vlan:%d)\n",
+ source ? source->dev->name : br->dev->name, addr, vid);
fdb_delete(br, fdb);
}
@@ -583,9 +583,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
/* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
if (net_ratelimit())
- br_warn(br, "received packet on %s with "
- "own address as source address\n",
- source->dev->name);
+ br_warn(br, "received packet on %s with own "
+ "address as source address "
+ "(addr:%pM, vlan:%d)\n",
+ source->dev->name, addr, vid);
} else {
/* fastpath: update of existing entry */
if (unlikely(source != fdb->dst)) {
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace
From: Zach Brown @ 2016-10-11 21:29 UTC (permalink / raw)
To: Stephen Hemminger
Cc: devel, florian.c.schilhabel, f.fainelli, andrew, netdev,
linux-kernel, rpurdie, gregkh, Larry.Finger, j.anaszewski,
linux-leds, mlindner
In-Reply-To: <20161011141407.3b233e83@xeon-e3>
On Tue, Oct 11, 2016 at 02:14:07PM -0700, Stephen Hemminger wrote:
> On Tue, 11 Oct 2016 15:26:18 -0500
> Zach Brown <zach.brown@ni.com> wrote:
>
> > Adding led support for phy causes namespace conflicts for some
> > phy drivers.
> >
> > The marvel skge driver declared an enum for representing the states of
> > Link LED Register. The enum contained constant LED_OFF which conflicted
> > with declartation found in linux/leds.h.
> > LED_OFF changed to LED_REG_OFF
> > Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
> > for consistency.
> >
> > Signed-off-by: Zach Brown <zach.brown@ni.com>
>
> Sure, that's fine but not sure why skge would be including linux/leds.h
> anyway.
It's pretty convoluted. Here's the chain of includes.
skge -> netdevice -> dsa -> phy -> phy_led_triggers -> leds
^ permalink raw reply
* Re: [PATCH v4 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace
From: Stephen Hemminger @ 2016-10-11 21:14 UTC (permalink / raw)
To: Zach Brown
Cc: devel, florian.c.schilhabel, f.fainelli, andrew, netdev,
linux-kernel, rpurdie, gregkh, Larry.Finger, j.anaszewski,
linux-leds, mlindner
In-Reply-To: <1476217580-21229-2-git-send-email-zach.brown@ni.com>
On Tue, 11 Oct 2016 15:26:18 -0500
Zach Brown <zach.brown@ni.com> wrote:
> Adding led support for phy causes namespace conflicts for some
> phy drivers.
>
> The marvel skge driver declared an enum for representing the states of
> Link LED Register. The enum contained constant LED_OFF which conflicted
> with declartation found in linux/leds.h.
> LED_OFF changed to LED_REG_OFF
> Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
> for consistency.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
Sure, that's fine but not sure why skge would be including linux/leds.h
anyway.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Lawrence Brakmo @ 2016-10-11 21:08 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <CAK6E8=eoK88K0APyO04LtefaaDg6qRtHgHcKgMjiDvdmNCkNng@mail.gmail.com>
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-C8ltkgyx1u_
>g&m=Q4nONH7kQ5AvQguw9UxpcHd79jfdDdrXj1YSJs7Ezhk&s=MA4fWBLMTGgRS0eGvBjxf7BJ
>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.
>
>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] netvsc: fix checksum on UDP IPV6
From: Stephen Hemminger @ 2016-10-11 21:03 UTC (permalink / raw)
To: David Miller, K. Y. Srinivasan, Haiyang Zhang; +Cc: netdev
The software calculation of UDP checksum in Netvsc driver was
only handling IPv4 case. By using skb_checksum_help() instead
all protocols can be handled. Rearrange code to eliminate goto
and look like other drivers.
This is a temporary solution; recent versions of Window Server etc
do support UDP checksum offload, just need to do the appropriate negotiation
with host to validate before using. This will be done in later patch.
Please queue this for -stable as well.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 72 +++++++++++++----------------------------
1 file changed, 22 insertions(+), 50 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 52eeb2f..9570d21 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -36,6 +36,7 @@
#include <net/arp.h>
#include <net/route.h>
#include <net/sock.h>
+#include <net/udp.h>
#include <net/pkt_sched.h>
#include "hyperv_net.h"
@@ -442,8 +443,6 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
}
net_trans_info = get_net_transport_info(skb, &hdr_offset);
- if (net_trans_info == TRANSPORT_INFO_NOT_IP)
- goto do_send;
/*
* Setup the sendside checksum offload only if this is not a
@@ -478,56 +477,29 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
}
lso_info->lso_v2_transmit.tcp_header_offset = hdr_offset;
lso_info->lso_v2_transmit.mss = skb_shinfo(skb)->gso_size;
- goto do_send;
- }
-
- if ((skb->ip_summed == CHECKSUM_NONE) ||
- (skb->ip_summed == CHECKSUM_UNNECESSARY))
- goto do_send;
-
- rndis_msg_size += NDIS_CSUM_PPI_SIZE;
- ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE,
- TCPIP_CHKSUM_PKTINFO);
-
- csum_info = (struct ndis_tcp_ip_checksum_info *)((void *)ppi +
- ppi->ppi_offset);
-
- if (net_trans_info & (INFO_IPV4 << 16))
- csum_info->transmit.is_ipv4 = 1;
- else
- csum_info->transmit.is_ipv6 = 1;
-
- if (net_trans_info & INFO_TCP) {
- csum_info->transmit.tcp_checksum = 1;
- csum_info->transmit.tcp_header_offset = hdr_offset;
- } else if (net_trans_info & INFO_UDP) {
- /* UDP checksum offload is not supported on ws2008r2.
- * Furthermore, on ws2012 and ws2012r2, there are some
- * issues with udp checksum offload from Linux guests.
- * (these are host issues).
- * For now compute the checksum here.
- */
- struct udphdr *uh;
- u16 udp_len;
-
- ret = skb_cow_head(skb, 0);
- if (ret)
- goto no_memory;
-
- uh = udp_hdr(skb);
- udp_len = ntohs(uh->len);
- uh->check = 0;
- uh->check = csum_tcpudp_magic(ip_hdr(skb)->saddr,
- ip_hdr(skb)->daddr,
- udp_len, IPPROTO_UDP,
- csum_partial(uh, udp_len, 0));
- if (uh->check == 0)
- uh->check = CSUM_MANGLED_0;
-
- csum_info->transmit.udp_checksum = 0;
+ } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (net_trans_info & INFO_TCP) {
+ rndis_msg_size += NDIS_CSUM_PPI_SIZE;
+ ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE,
+ TCPIP_CHKSUM_PKTINFO);
+
+ csum_info = (struct ndis_tcp_ip_checksum_info *)((void *)ppi +
+ ppi->ppi_offset);
+
+ if (net_trans_info & (INFO_IPV4 << 16))
+ csum_info->transmit.is_ipv4 = 1;
+ else
+ csum_info->transmit.is_ipv6 = 1;
+
+ csum_info->transmit.tcp_checksum = 1;
+ csum_info->transmit.tcp_header_offset = hdr_offset;
+ } else {
+ /* UDP checksum (and other) offload is not supported. */
+ if (skb_checksum_help(skb))
+ goto drop;
+ }
}
-do_send:
/* Start filling in the page buffers with the rndis hdr */
rndis_msg->msg_len += rndis_msg_size;
packet->total_data_buflen = rndis_msg->msg_len;
--
2.9.3
^ permalink raw reply related
* Re: [RFC v3 3/3] phy,leds: add support for led triggers on phy link state change
From: Andrew Lunn @ 2016-10-11 20:56 UTC (permalink / raw)
To: Florian Fainelli
Cc: Zach Brown, mlindner, stephen, netdev, linux-kernel, devel,
florian.c.schilhabel, Larry.Finger, gregkh, rpurdie, j.anaszewski,
linux-leds
In-Reply-To: <1eb3afdb-0236-65e3-64be-09cc6eeda533@gmail.com>
> Andrew, are you happy with this implementation?
Sorry, been to busy with other things to follow the discussion.
What would be nice to see is a comment about how the link to LEDs in
the PHYs is made. Often there is a couple of LEDs in the RJ45 socket
driven by the PHY. They can show link, speed, packet Rx/Tx, etc. How
are these triggers related to these LEDs?
Andrew
^ permalink raw reply
* [PATCH net] ip6_tunnel: fix ip6_tnl_lookup
From: Vadim Fedorenko @ 2016-10-11 19:47 UTC (permalink / raw)
To: netdev
The commit ea3dc9601bda ("ip6_tunnel: Add support for wildcard tunnel
endpoints.") introduces support for wildcards in tunnels endpoints,
but in some rare circumstances ip6_tnl_lookup selects wrong tunnel
interface relying only on source or destination address of the packet
and not checking presence of wildcard in tunnels endpoints. Later in
ip6_tnl_rcv this packets can be dicarded because of difference in
ipproto even if fallback device have proper ipproto configuration.
This patch adds checks of wildcard endpoint in tunnel avoiding such
behavior
Fixes: ea3dc9601bda ("ip6_tunnel: Add support for wildcard tunnel
endpoints.")
Signed-off-by: Vadim Fedorenko <junk@yandex-team.ru>
---
net/ipv6/ip6_tunnel.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 6a66adb..5692d6b 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -157,6 +157,7 @@ ip6_tnl_lookup(struct net *net, const struct in6_addr *remote, const struct in6_
hash = HASH(&any, local);
for_each_ip6_tunnel_rcu(ip6n->tnls_r_l[hash]) {
if (ipv6_addr_equal(local, &t->parms.laddr) &&
+ ipv6_addr_any(&t->parms.raddr) &&
(t->dev->flags & IFF_UP))
return t;
}
@@ -164,6 +165,7 @@ ip6_tnl_lookup(struct net *net, const struct in6_addr *remote, const struct in6_
hash = HASH(remote, &any);
for_each_ip6_tunnel_rcu(ip6n->tnls_r_l[hash]) {
if (ipv6_addr_equal(remote, &t->parms.raddr) &&
+ ipv6_addr_any(&t->parms.laddr) &&
(t->dev->flags & IFF_UP))
return t;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v4 3/3] net: phy: leds: add support for led triggers on phy link state change
From: Zach Brown @ 2016-10-11 20:26 UTC (permalink / raw)
To: f.fainelli
Cc: devel, florian.c.schilhabel, andrew, netdev, linux-kernel,
rpurdie, gregkh, Larry.Finger, j.anaszewski, linux-leds, mlindner
In-Reply-To: <1476217580-21229-1-git-send-email-zach.brown@ni.com>
From: Josh Cartwright <josh.cartwright@ni.com>
Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device. There is
one LED trigger per link-speed, per-phy.
This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.
Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/net/phy/Kconfig | 13 +++-
drivers/net/phy/Makefile | 1 +
drivers/net/phy/phy.c | 1 +
drivers/net/phy/phy_device.c | 4 ++
drivers/net/phy/phy_led_triggers.c | 121 +++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 9 +++
include/linux/phy_led_triggers.h | 52 ++++++++++++++++
7 files changed, 200 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/phy/phy_led_triggers.c
create mode 100644 include/linux/phy_led_triggers.h
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5078a0d..4fd912d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -25,6 +25,18 @@ config MDIO_BCM_IPROC
This module provides a driver for the MDIO busses found in the
Broadcom iProc SoC's.
+config LED_TRIGGER_PHY
+ bool "Support LED triggers for tracking link state"
+ depends on LEDS_TRIGGERS
+ ---help---
+ Adds support for a set of LED trigger events per-PHY. Link
+ state change will trigger the events, for consumption by an
+ LED class driver. There are triggers for each link speed,
+ and are of the form:
+ <mii bus id>:<phy>:<speed>
+
+ Where speed is one of: 10Mbps, 100Mbps, 1Gbps, 2.5Gbps, or 10Gbps.
+
config MDIO_BCM_UNIMAC
tristate "Broadcom UniMAC MDIO bus controller"
depends on HAS_IOMEM
@@ -40,7 +52,6 @@ config MDIO_BITBANG
This module implements the MDIO bus protocol in software,
for use by low level drivers that export the ability to
drive the relevant pins.
-
If in doubt, say N.
config MDIO_BUS_MUX
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
libphy-y := phy.o phy_device.o mdio_bus.o mdio_device.o
libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
obj-$(CONFIG_PHYLIB) += libphy.o
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f5721db..e5f9fee7 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -896,6 +896,7 @@ EXPORT_SYMBOL(phy_start);
static void phy_adjust_link(struct phy_device *phydev)
{
phydev->adjust_link(phydev->attached_dev);
+ phy_led_trigger_change_speed(phydev);
}
/**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..4671c13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
#include <linux/mii.h>
#include <linux/ethtool.h>
#include <linux/phy.h>
+#include <linux/phy_led_triggers.h>
#include <linux/mdio.h>
#include <linux/io.h>
#include <linux/uaccess.h>
@@ -57,6 +58,7 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
static void phy_device_release(struct device *dev)
{
+ phy_led_triggers_unregister(to_phy_device(dev));
kfree(to_phy_device(dev));
}
@@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
dev->state = PHY_DOWN;
+ phy_led_triggers_register(dev);
+
mutex_init(&dev->lock);
INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
INIT_WORK(&dev->phy_queue, phy_change);
diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 0000000..32326d7
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,121 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/leds.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy,
+ unsigned int speed)
+{
+ switch (speed) {
+ case SPEED_10:
+ return &phy->phy_led_trigger[0];
+ case SPEED_100:
+ return &phy->phy_led_trigger[1];
+ case SPEED_1000:
+ return &phy->phy_led_trigger[2];
+ case SPEED_2500:
+ return &phy->phy_led_trigger[3];
+ case SPEED_10000:
+ return &phy->phy_led_trigger[4];
+ default:
+ return NULL;
+ }
+}
+
+void phy_led_trigger_change_speed(struct phy_device *phy)
+{
+ struct phy_led_trigger *plt;
+
+ if (!phy->link)
+ goto out_change_speed;
+
+ if (phy->speed == 0)
+ return;
+
+ plt = phy_speed_to_led_trigger(phy, phy->speed);
+ if (!plt) {
+ netdev_alert(phy->attached_dev,
+ "Unsupported trigger speed %u (update phy_led_trigger.c)\n",
+ phy->speed);
+ goto out_change_speed;
+ }
+
+ if (plt != phy->last_triggered) {
+ led_trigger_event(&phy->last_triggered->trigger, LED_OFF);
+ led_trigger_event(&plt->trigger, LED_FULL);
+ phy->last_triggered = plt;
+ }
+ return;
+
+out_change_speed:
+ if (phy->last_triggered) {
+ led_trigger_event(&phy->last_triggered->trigger,
+ LED_OFF);
+ phy->last_triggered = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(phy_led_trigger_change_speed);
+
+static int phy_led_trigger_register(struct phy_device *phy,
+ struct phy_led_trigger *plt, int i)
+{
+ static const char * const name_suffix[] = {
+ "10Mbps",
+ "100Mbps",
+ "1Gbps",
+ "2.5Gbps",
+ "10Gbps",
+ };
+ snprintf(plt->name, sizeof(plt->name), PHY_ID_FMT ":%s",
+ phy->mdio.bus->id, phy->mdio.addr, name_suffix[i]);
+ plt->trigger.name = plt->name;
+ return led_trigger_register(&plt->trigger);
+}
+
+static void phy_led_trigger_unregister(struct phy_led_trigger *plt)
+{
+ led_trigger_unregister(&plt->trigger);
+}
+
+int phy_led_triggers_register(struct phy_device *phy)
+{
+ int i, err;
+
+ for (i = 0; i < ARRAY_SIZE(phy->phy_led_trigger); i++) {
+ err = phy_led_trigger_register(phy, &phy->phy_led_trigger[i],
+ i);
+ if (err)
+ goto out_unreg;
+ }
+
+ phy->last_triggered = NULL;
+ phy_led_trigger_change_speed(phy);
+
+ return 0;
+
+out_unreg:
+ while (i--)
+ phy_led_trigger_unregister(&phy->phy_led_trigger[i]);
+ return err;
+}
+EXPORT_SYMBOL_GPL(phy_led_triggers_register);
+
+void phy_led_triggers_unregister(struct phy_device *phy)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(phy->phy_led_trigger); i++)
+ phy_led_trigger_unregister(&phy->phy_led_trigger[i]);
+}
+EXPORT_SYMBOL_GPL(phy_led_triggers_unregister);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e25f183..6af4d6d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -25,6 +25,7 @@
#include <linux/timer.h>
#include <linux/workqueue.h>
#include <linux/mod_devicetable.h>
+#include <linux/phy_led_triggers.h>
#include <linux/atomic.h>
@@ -405,6 +406,14 @@ struct phy_device {
int link_timeout;
+#ifdef CONFIG_LED_TRIGGER_PHY
+ /*
+ * A led_trigger per SPEED_*
+ */
+ struct phy_led_trigger phy_led_trigger[PHY_LINK_LED_MAX_TRIGGERS];
+ struct phy_led_trigger *last_triggered;
+#endif
+
/*
* Interrupt number for this PHY
* -1 means no interrupt
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h
new file mode 100644
index 0000000..dfea5d6
--- /dev/null
+++ b/include/linux/phy_led_triggers.h
@@ -0,0 +1,52 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef __PHY_LED_TRIGGERS
+#define __PHY_LED_TRIGGERS
+
+struct phy_device;
+
+#ifdef CONFIG_LED_TRIGGER_PHY
+
+#include <linux/leds.h>
+#include <linux/phy.h>
+
+#define PHY_LINK_LED_MAX_TRIGGERS 5
+#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 7
+#define PHY_MII_BUS_ID_SIZE (20 - 3)
+
+#define PHY_LINK_LED_TRIGGER_NAME_SIZE (PHY_MII_BUS_ID_SIZE + \
+ FIELD_SIZEOF(struct mdio_device, addr)+\
+ PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE)
+
+struct phy_led_trigger {
+ struct led_trigger trigger;
+ char name[PHY_LINK_LED_TRIGGER_NAME_SIZE];
+};
+
+
+extern int phy_led_triggers_register(struct phy_device *phy);
+extern void phy_led_triggers_unregister(struct phy_device *phy);
+extern void phy_led_trigger_change_speed(struct phy_device *phy);
+
+#else
+
+static inline int phy_led_triggers_register(struct phy_device *phy)
+{
+ return 0;
+}
+static inline void phy_led_triggers_unregister(struct phy_device *phy) { }
+static inline void phy_led_trigger_change_speed(struct phy_device *phy) { }
+
+#endif
+
+#endif
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/3] net: phy: Encapsulate actions performed during link state changes into function phy_adjust_link
From: Zach Brown @ 2016-10-11 20:26 UTC (permalink / raw)
To: f.fainelli
Cc: devel, florian.c.schilhabel, andrew, netdev, linux-kernel,
rpurdie, gregkh, Larry.Finger, j.anaszewski, linux-leds, mlindner
In-Reply-To: <1476217580-21229-1-git-send-email-zach.brown@ni.com>
During phy state machine state transitions some set of actions should
occur whenever the link state changes. These actions should be
encapsulated into a single function
This patch adds the phy_adjust_link function, which is called whenever
phydev->adjust_link would have been called before. Actions that should
occur whenever the phy link is adjusted can now be added to the
phy_adjust_link function.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/net/phy/phy.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..f5721db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -893,6 +893,11 @@ void phy_start(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_start);
+static void phy_adjust_link(struct phy_device *phydev)
+{
+ phydev->adjust_link(phydev->attached_dev);
+}
+
/**
* phy_state_machine - Handle the state machine
* @work: work_struct that describes the work to be done
@@ -935,7 +940,7 @@ void phy_state_machine(struct work_struct *work)
if (!phydev->link) {
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
break;
}
@@ -948,7 +953,7 @@ void phy_state_machine(struct work_struct *work)
if (err > 0) {
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -975,7 +980,7 @@ void phy_state_machine(struct work_struct *work)
}
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
}
break;
case PHY_FORCING:
@@ -991,7 +996,7 @@ void phy_state_machine(struct work_struct *work)
needs_aneg = true;
}
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1020,7 +1025,7 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_off(phydev->attached_dev);
}
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1030,7 +1035,7 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
do_suspend = true;
}
break;
@@ -1054,7 +1059,7 @@ void phy_state_machine(struct work_struct *work)
} else {
phydev->state = PHY_NOLINK;
}
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
} else {
phydev->state = PHY_AN;
phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1070,7 +1075,7 @@ void phy_state_machine(struct work_struct *work)
} else {
phydev->state = PHY_NOLINK;
}
- phydev->adjust_link(phydev->attached_dev);
+ phy_adjust_link(phydev);
}
break;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v4 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace
From: Zach Brown @ 2016-10-11 20:26 UTC (permalink / raw)
To: f.fainelli
Cc: devel, florian.c.schilhabel, andrew, netdev, linux-kernel,
rpurdie, gregkh, Larry.Finger, j.anaszewski, linux-leds, mlindner
In-Reply-To: <1476217580-21229-1-git-send-email-zach.brown@ni.com>
Adding led support for phy causes namespace conflicts for some
phy drivers.
The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF
Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
for consistency.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/net/ethernet/marvell/skge.c | 6 +++---
drivers/net/ethernet/marvell/skge.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 7173836..783df01 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1048,7 +1048,7 @@ static const char *skge_pause(enum pause_status status)
static void skge_link_up(struct skge_port *skge)
{
skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG),
- LED_BLK_OFF|LED_SYNC_OFF|LED_ON);
+ LED_BLK_OFF|LED_SYNC_OFF|LED_REG_ON);
netif_carrier_on(skge->netdev);
netif_wake_queue(skge->netdev);
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
static void skge_link_down(struct skge_port *skge)
{
- skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+ skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
- skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+ skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h b/drivers/net/ethernet/marvell/skge.h
index a2eb341..3ea151f 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -662,8 +662,8 @@ enum {
LED_BLK_OFF = 1<<4, /* Link LED Blinking Off */
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF = 1<<2, /* Disable Sync Wire Input */
- LED_ON = 1<<1, /* switch LED on */
- LED_OFF = 1<<0, /* switch LED off */
+ LED_REG_ON = 1<<1, /* switch LED on */
+ LED_REG_OFF = 1<<0, /* switch LED off */
};
/* Receive GMAC FIFO (YUKON) */
--
2.7.4
^ permalink raw reply related
* [PATCH v4 0/3] Add support for led triggers on phy link state change
From: Zach Brown @ 2016-10-11 20:26 UTC (permalink / raw)
To: f.fainelli
Cc: devel, florian.c.schilhabel, andrew, netdev, linux-kernel,
rpurdie, gregkh, Larry.Finger, j.anaszewski, linux-leds, mlindner
Fix skge driver that declared enum contants that conflicted with enum
constants in linux/leds.h
Create function that encapsulates actions taken during the adjust phy link step
of phy state changes.
Add support for led triggers on phy link state changes by adding
a config option. When set the config option will create a set of led triggers
for each phy device. Users can use the led triggers to represent link state
changes on the phy.
v2:
* New patch that creates phy_adjust_link function to encapsulate actions taken
when adjusting phy link during phy state changes
* led trigger speed strings changed to match existing phy speed strings
* New function that maps speeds to led triggers
* Replace magic constants with definitions when declaring trigger name
buffer and number of triggers.
v3:
* Changed LED_ON to LED_REG_ON in skge driver to avoid possible future
conflict and improve consistency.
* Dropped rtl8712 patch that was accepted separately.
v4:
* tweaked commit message
Josh Cartwright (1):
net: phy: leds: add support for led triggers on phy link state change
Zach Brown (2):
skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid
conflicts with leds namespace
net: phy: Encapsulate actions performed during link state changes into
function phy_adjust_link
drivers/net/ethernet/marvell/skge.c | 6 +-
drivers/net/ethernet/marvell/skge.h | 4 +-
drivers/net/phy/Kconfig | 13 +++-
drivers/net/phy/Makefile | 1 +
drivers/net/phy/phy.c | 22 ++++---
drivers/net/phy/phy_device.c | 4 ++
drivers/net/phy/phy_led_triggers.c | 121 ++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 9 +++
include/linux/phy_led_triggers.h | 52 ++++++++++++++++
9 files changed, 218 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/phy/phy_led_triggers.c
create mode 100644 include/linux/phy_led_triggers.h
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Saeed Mahameed @ 2016-10-11 7:44 UTC (permalink / raw)
To: Tom Herbert, davem, netdev; +Cc: kernel-team
In-Reply-To: <20161011042222.2366145-1-tom@herbertland.com>
On 10/11/2016 01:22 PM, Tom Herbert wrote:
> I am hitting this in mlx5:
>
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function
> ‘reclaim_pages_cmd.clone.0’:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:346: error: call
> to ‘__compiletime_assert_346’ declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_out, pas[i]) % 64
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function ‘give_pages’:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:291: error: call
> to ‘__compiletime_assert_291’ declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_in, pas[i]) % 64
>
> Problem is that this is doing a BUILD_BUG_ON on a non-constant
> expression because of trying to take offset of pas[i] in the
> structure.
>
> Fix is to create MLX5_SET64_VCHK that takes an additional argument
> that is a constant. There are two callers of MLX5_SET64 that are
> trying to get a variable offset, change those to call MLX5_SET64_VCHK
> passing pas[0] as the argument to use in the offset check.
>
> Fixes: a533ed5e179cd ("net/mlx5: Pages management commands via mlx5 ifc")
> Signed-off-by: Tom Herbert <tom@herbertland.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply
* [PATCH net] Revert "net: Add driver helper functions to determine checksum offloadability"
From: Stephen Hemminger @ 2016-10-11 20:04 UTC (permalink / raw)
To: David Miller, Tom Herbert; +Cc: netdev
This reverts commit 6ae23ad36253a8033c5714c52b691b84456487c5.
The code has been in kernel since 4.4 but there are no in tree
code that uses. Unused code is broken code, remove it.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
include/linux/netdevice.h | 78 --------------------------
net/core/dev.c | 136 ----------------------------------------------
2 files changed, 214 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 136ae6bb..793155e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2649,71 +2649,6 @@ static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
remcsum_unadjust((__sum16 *)ptr, grc->delta);
}
-struct skb_csum_offl_spec {
- __u16 ipv4_okay:1,
- ipv6_okay:1,
- encap_okay:1,
- ip_options_okay:1,
- ext_hdrs_okay:1,
- tcp_okay:1,
- udp_okay:1,
- sctp_okay:1,
- vlan_okay:1,
- no_encapped_ipv6:1,
- no_not_encapped:1;
-};
-
-bool __skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help);
-
-static inline bool skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help)
-{
- if (skb->ip_summed != CHECKSUM_PARTIAL)
- return false;
-
- return __skb_csum_offload_chk(skb, spec, csum_encapped, csum_help);
-}
-
-static inline bool skb_csum_offload_chk_help(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec)
-{
- bool csum_encapped;
-
- return skb_csum_offload_chk(skb, spec, &csum_encapped, true);
-}
-
-static inline bool skb_csum_off_chk_help_cmn(struct sk_buff *skb)
-{
- static const struct skb_csum_offl_spec csum_offl_spec = {
- .ipv4_okay = 1,
- .ip_options_okay = 1,
- .ipv6_okay = 1,
- .vlan_okay = 1,
- .tcp_okay = 1,
- .udp_okay = 1,
- };
-
- return skb_csum_offload_chk_help(skb, &csum_offl_spec);
-}
-
-static inline bool skb_csum_off_chk_help_cmn_v4_only(struct sk_buff *skb)
-{
- static const struct skb_csum_offl_spec csum_offl_spec = {
- .ipv4_okay = 1,
- .ip_options_okay = 1,
- .tcp_okay = 1,
- .udp_okay = 1,
- .vlan_okay = 1,
- };
-
- return skb_csum_offload_chk_help(skb, &csum_offl_spec);
-}
-
static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
unsigned short type,
const void *daddr, const void *saddr,
@@ -3957,19 +3892,6 @@ static inline bool can_checksum_protocol(netdev_features_t features,
}
}
-/* Map an ethertype into IP protocol if possible */
-static inline int eproto_to_ipproto(int eproto)
-{
- switch (eproto) {
- case htons(ETH_P_IP):
- return IPPROTO_IP;
- case htons(ETH_P_IPV6):
- return IPPROTO_IPV6;
- default:
- return -1;
- }
-}
-
#ifdef CONFIG_BUG
void netdev_rx_csum_fault(struct net_device *dev);
#else
diff --git a/net/core/dev.c b/net/core/dev.c
index f1fe26f..593f427 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -139,7 +139,6 @@
#include <linux/errqueue.h>
#include <linux/hrtimer.h>
#include <linux/netfilter_ingress.h>
-#include <linux/sctp.h>
#include <linux/crash_dump.h>
#include "net-sysfs.h"
@@ -2492,141 +2491,6 @@ out:
}
EXPORT_SYMBOL(skb_checksum_help);
-/* skb_csum_offload_check - Driver helper function to determine if a device
- * with limited checksum offload capabilities is able to offload the checksum
- * for a given packet.
- *
- * Arguments:
- * skb - sk_buff for the packet in question
- * spec - contains the description of what device can offload
- * csum_encapped - returns true if the checksum being offloaded is
- * encpasulated. That is it is checksum for the transport header
- * in the inner headers.
- * checksum_help - when set indicates that helper function should
- * call skb_checksum_help if offload checks fail
- *
- * Returns:
- * true: Packet has passed the checksum checks and should be offloadable to
- * the device (a driver may still need to check for additional
- * restrictions of its device)
- * false: Checksum is not offloadable. If checksum_help was set then
- * skb_checksum_help was called to resolve checksum for non-GSO
- * packets and when IP protocol is not SCTP
- */
-bool __skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help)
-{
- struct iphdr *iph;
- struct ipv6hdr *ipv6;
- void *nhdr;
- int protocol;
- u8 ip_proto;
-
- if (skb->protocol == htons(ETH_P_8021Q) ||
- skb->protocol == htons(ETH_P_8021AD)) {
- if (!spec->vlan_okay)
- goto need_help;
- }
-
- /* We check whether the checksum refers to a transport layer checksum in
- * the outermost header or an encapsulated transport layer checksum that
- * corresponds to the inner headers of the skb. If the checksum is for
- * something else in the packet we need help.
- */
- if (skb_checksum_start_offset(skb) == skb_transport_offset(skb)) {
- /* Non-encapsulated checksum */
- protocol = eproto_to_ipproto(vlan_get_protocol(skb));
- nhdr = skb_network_header(skb);
- *csum_encapped = false;
- if (spec->no_not_encapped)
- goto need_help;
- } else if (skb->encapsulation && spec->encap_okay &&
- skb_checksum_start_offset(skb) ==
- skb_inner_transport_offset(skb)) {
- /* Encapsulated checksum */
- *csum_encapped = true;
- switch (skb->inner_protocol_type) {
- case ENCAP_TYPE_ETHER:
- protocol = eproto_to_ipproto(skb->inner_protocol);
- break;
- case ENCAP_TYPE_IPPROTO:
- protocol = skb->inner_protocol;
- break;
- }
- nhdr = skb_inner_network_header(skb);
- } else {
- goto need_help;
- }
-
- switch (protocol) {
- case IPPROTO_IP:
- if (!spec->ipv4_okay)
- goto need_help;
- iph = nhdr;
- ip_proto = iph->protocol;
- if (iph->ihl != 5 && !spec->ip_options_okay)
- goto need_help;
- break;
- case IPPROTO_IPV6:
- if (!spec->ipv6_okay)
- goto need_help;
- if (spec->no_encapped_ipv6 && *csum_encapped)
- goto need_help;
- ipv6 = nhdr;
- nhdr += sizeof(*ipv6);
- ip_proto = ipv6->nexthdr;
- break;
- default:
- goto need_help;
- }
-
-ip_proto_again:
- switch (ip_proto) {
- case IPPROTO_TCP:
- if (!spec->tcp_okay ||
- skb->csum_offset != offsetof(struct tcphdr, check))
- goto need_help;
- break;
- case IPPROTO_UDP:
- if (!spec->udp_okay ||
- skb->csum_offset != offsetof(struct udphdr, check))
- goto need_help;
- break;
- case IPPROTO_SCTP:
- if (!spec->sctp_okay ||
- skb->csum_offset != offsetof(struct sctphdr, checksum))
- goto cant_help;
- break;
- case NEXTHDR_HOP:
- case NEXTHDR_ROUTING:
- case NEXTHDR_DEST: {
- u8 *opthdr = nhdr;
-
- if (protocol != IPPROTO_IPV6 || !spec->ext_hdrs_okay)
- goto need_help;
-
- ip_proto = opthdr[0];
- nhdr += (opthdr[1] + 1) << 3;
-
- goto ip_proto_again;
- }
- default:
- goto need_help;
- }
-
- /* Passed the tests for offloading checksum */
- return true;
-
-need_help:
- if (csum_help && !skb_shinfo(skb)->gso_size)
- skb_checksum_help(skb);
-cant_help:
- return false;
-}
-EXPORT_SYMBOL(__skb_csum_offload_chk);
-
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
__be16 type = skb->protocol;
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox