* Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: David Miller @ 2017-12-07 20:04 UTC (permalink / raw)
To: marcelo.leitner
Cc: laoar.shao, songliubraving, kuznet, yoshfuji, rostedt, bgregg,
netdev, linux-kernel
In-Reply-To: <20171207200245.GQ13341@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 7 Dec 2017 18:02:46 -0200
> Dave?
Yeah that will untangle this mess much more nicely.
^ permalink raw reply
* Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: Marcelo Ricardo Leitner @ 2017-12-07 20:02 UTC (permalink / raw)
To: Yafang Shao
Cc: davem, songliubraving, kuznet, yoshfuji, rostedt, bgregg, netdev,
linux-kernel
In-Reply-To: <1512655842-17784-1-git-send-email-laoar.shao@gmail.com>
On Thu, Dec 07, 2017 at 02:10:42PM +0000, Yafang Shao wrote:
> The TCP/IP transition from TCP_LISTEN to TCP_SYN_RECV and some other
> transitions are not traced with tcp_set_state tracepoint.
>
> In order to trace the whole tcp lifespans, two helpers are introduced,
> void sk_set_state(struct sock *sk, int state);
> void sk_state_store(struct sock *sk, int newstate);
>
> When do TCP/IP state transition, we should use these two helpers or use
> tcp_set_state() other than assigning a value to sk_state directly.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> v4->v5: Trace only TCP sockets, whatever it is stream socket or raw socket.
> v3->v4: Do not trace DCCP socket
> v2->v3: Per suggestion from Marcelo Ricardo Leitner, inverting __
> to sk_state_store.
> ---
> include/net/sock.h | 8 ++++++--
> net/core/sock.c | 15 +++++++++++++++
> net/ipv4/inet_connection_sock.c | 5 +++--
> net/ipv4/inet_hashtables.c | 2 +-
> net/ipv4/tcp.c | 2 +-
> 5 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 79e1a2c..1cf7685 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2349,18 +2349,22 @@ static inline int sk_state_load(const struct sock *sk)
> }
>
> /**
> - * sk_state_store - update sk->sk_state
> + * __sk_state_store - update sk->sk_state
> * @sk: socket pointer
> * @newstate: new state
> *
> * Paired with sk_state_load(). Should be used in contexts where
> * state change might impact lockless readers.
> */
> -static inline void sk_state_store(struct sock *sk, int newstate)
> +static inline void __sk_state_store(struct sock *sk, int newstate)
> {
> smp_store_release(&sk->sk_state, newstate);
> }
>
> +/* For tcp_set_state tracepoint */
> +void sk_state_store(struct sock *sk, int newstate);
> +void sk_set_state(struct sock *sk, int state);
> +
> void sock_enable_timestamp(struct sock *sk, int flag);
> int sock_get_timestamp(struct sock *, struct timeval __user *);
> int sock_get_timestampns(struct sock *, struct timespec __user *);
> diff --git a/net/core/sock.c b/net/core/sock.c
> index c0b5b2f..61841a2 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -138,6 +138,7 @@
> #include <net/sock_reuseport.h>
>
> #include <trace/events/sock.h>
> +#include <trace/events/tcp.h>
>
> #include <net/tcp.h>
> #include <net/busy_poll.h>
> @@ -2859,6 +2860,20 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
> }
> EXPORT_SYMBOL(sock_get_timestampns);
>
> +void sk_state_store(struct sock *sk, int newstate)
> +{
> + if (sk->sk_protocol == IPPROTO_TCP)
> + trace_tcp_set_state(sk, sk->sk_state, newstate);
I think this is going in the wrong way. When Dave said to not define a
sock generic function in tcp code on v3, you moved it all from tcp.h
to sock.h. But now sock.h gets to deal with more tcp code, which also
isn't nice.
Instead, if you move it back to tcp.h but rename the function to
tcp_state_store (instead of the original sk_state_store), it won't be
a generic sock code and will fit nicely into tcp.h.
You may then even keep sk_state_store() as it is now, and just define
tcp_state_store():
tcp_state_store()
{
trace_tcp_...();
sk_state_store();
}
Making it very clear that this code is only to be used by tcp stack.
> + __sk_state_store(sk, newstate);
> +}
> +
> +void sk_set_state(struct sock *sk, int state)
Same about this one.
Dave?
> +{
> + if (sk->sk_protocol == IPPROTO_TCP)
> + trace_tcp_set_state(sk, sk->sk_state, state);
> + sk->sk_state = state;
> +}
> +
> void sock_enable_timestamp(struct sock *sk, int flag)
> {
> if (!sock_flag(sk, flag)) {
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 4ca46dc..41f9c87 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
> if (newsk) {
> struct inet_connection_sock *newicsk = inet_csk(newsk);
>
> - newsk->sk_state = TCP_SYN_RECV;
> + sk_set_state(newsk, TCP_SYN_RECV);
> newicsk->icsk_bind_hash = NULL;
>
> inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
> @@ -888,7 +888,8 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
> return 0;
> }
>
> - sk->sk_state = TCP_CLOSE;
> + sk_set_state(sk, TCP_CLOSE);
> +
> return err;
> }
> EXPORT_SYMBOL_GPL(inet_csk_listen_start);
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index f6f5810..5973693 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
> sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
> } else {
> percpu_counter_inc(sk->sk_prot->orphan_count);
> - sk->sk_state = TCP_CLOSE;
> + sk_set_state(sk, TCP_CLOSE);
> sock_set_flag(sk, SOCK_DEAD);
> inet_csk_destroy_sock(sk);
> }
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1803116..ac98dc6 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2065,7 +2065,7 @@ void tcp_set_state(struct sock *sk, int state)
> /* Change state AFTER socket is unhashed to avoid closed
> * socket sitting in hash tables.
> */
> - sk_state_store(sk, state);
> + __sk_state_store(sk, state);
>
> #ifdef STATE_TRACE
> SOCK_DEBUG(sk, "TCP sk=%p, State %s -> %s\n", sk, statename[oldstate], statename[state]);
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
From: Yang Shi @ 2017-12-07 20:02 UTC (permalink / raw)
To: Jon Maloy, linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
linux-crypto@vger.kernel.org, netdev@vger.kernel.org, Ying Xue,
David S. Miller
In-Reply-To: <AM4PR07MB17147A3C4885EE59CFA58BDA9A330@AM4PR07MB1714.eurprd07.prod.outlook.com>
On 12/7/17 11:20 AM, Jon Maloy wrote:
>
>
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>> owner@vger.kernel.org] On Behalf Of Yang Shi
>> Sent: Thursday, December 07, 2017 14:16
>> To: linux-kernel@vger.kernel.org
>> Cc: linux-mm@kvack.org; linux-fsdevel@vger.kernel.org; linux-
>> crypto@vger.kernel.org; netdev@vger.kernel.org; Jon Maloy
>> <jon.maloy@ericsson.com>; Ying Xue <ying.xue@windriver.com>; David S.
>> Miller <davem@davemloft.net>
>> Subject: Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
>>
>> Hi folks,
>>
>> Any comment on this one?
>
> If it compiles it is ok with me. Don't know why it was put there in the first place.
Yes, it does compile.
Yang
>
> ///jon
>
>>
>> Thanks,
>> Yang
>>
>>
>> On 11/17/17 3:02 PM, Yang Shi wrote:
>>> Preempt counter APIs have been split out, currently, hardirq.h just
>>> includes irq_enter/exit APIs which are not used by TIPC at all.
>>>
>>> So, remove the unused hardirq.h.
>>>
>>> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
>>> Cc: Jon Maloy <jon.maloy@ericsson.com>
>>> Cc: Ying Xue <ying.xue@windriver.com>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> ---
>>> net/tipc/core.h | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/net/tipc/core.h b/net/tipc/core.h index 5cc5398..099e072
>>> 100644
>>> --- a/net/tipc/core.h
>>> +++ b/net/tipc/core.h
>>> @@ -49,7 +49,6 @@
>>> #include <linux/uaccess.h>
>>> #include <linux/interrupt.h>
>>> #include <linux/atomic.h>
>>> -#include <asm/hardirq.h>
>>> #include <linux/netdevice.h>
>>> #include <linux/in.h>
>>> #include <linux/list.h>
>>>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net-next 0/6] cxgb4: collect hardware logs via ethtool
From: David Miller @ 2017-12-07 19:57 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <20171207.145606.1899010205120159321.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 07 Dec 2017 14:56:06 -0500 (EST)
> From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Date: Thu, 7 Dec 2017 14:33:36 +0530
>
>> Collect more hardware logs via ethtool --get-dump facility.
>>
>> Patch 1 collects on-chip memory layout information.
>>
>> Patch 2 collects on-chip MC memory dumps.
>>
>> Patch 3 collects HMA memory dump.
>>
>> Patch 4 evaluates and skips TX and RX payload regions in memory dumps.
>>
>> Patch 5 collects egress and ingress SGE queue contexts.
>>
>> Patch 6 collects PCIe configuration logs
>
> Series applied.
Actually, reverted, please fix this new build warning:
drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c: In function ‘cudbg_fill_meminfo’:
drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:261:33: warning: ‘size’ may be used uninitialized in this function [-Wmaybe-uninitialized]
md->limit = md->base + (size << 2) - 1;
~~~~~~^~~~~
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: correct error handling for chan create
From: Ivan Khoronzhuk @ 2017-12-07 19:56 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171206.163814.2298864765495270615.davem@davemloft.net>
On Wed, Dec 06, 2017 at 04:38:14PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Wed, 6 Dec 2017 16:54:09 +0200
>
> > @@ -3065,10 +3065,16 @@ static int cpsw_probe(struct platform_device *pdev)
> > }
> >
> > cpsw->txv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0);
> > + if (WARN_ON(IS_ERR(cpsw->txv[0].ch))) {
> > + dev_err(priv->dev, "error initializing tx dma channel\n");
> > + ret = PTR_ERR(cpsw->txv[0].ch);
> > + goto clean_dma_ret;
> > + }
> > +
>
> You're already emitting a proper dev_err() message, therefore WARN_ON()
> is a duplicate notification to the logs and therefore not appropriate.
Yes, will remove unneeded WARNs
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next 0/6] cxgb4: collect hardware logs via ethtool
From: David Miller @ 2017-12-07 19:56 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <cover.1512636113.git.rahul.lakkireddy@chelsio.com>
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Date: Thu, 7 Dec 2017 14:33:36 +0530
> Collect more hardware logs via ethtool --get-dump facility.
>
> Patch 1 collects on-chip memory layout information.
>
> Patch 2 collects on-chip MC memory dumps.
>
> Patch 3 collects HMA memory dump.
>
> Patch 4 evaluates and skips TX and RX payload regions in memory dumps.
>
> Patch 5 collects egress and ingress SGE queue contexts.
>
> Patch 6 collects PCIe configuration logs
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated
From: David Miller @ 2017-12-07 19:53 UTC (permalink / raw)
To: antoine.tenart
Cc: gregory.clement, thomas.petazzoni, miquel.raynal, nadavh, mw,
stefanc, ymarkman, netdev, linux-kernel
In-Reply-To: <20171207084903.27144-2-antoine.tenart@free-electrons.com>
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Thu, 7 Dec 2017 09:48:58 +0100
> This patch adds a check to only free the TSO header buffer when its
> allocation previously succeeded.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
No, please keep this as a failure to bring up.
Even if you emit a log message, it is completely unintuitive to
have netdev features change on the user just because of a memory
allocation failure.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: David Miller @ 2017-12-07 19:50 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171207194855.GA3022@khorivan>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Thu, 7 Dec 2017 21:48:56 +0200
> On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
>> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> Date: Wed, 6 Dec 2017 16:41:18 +0200
>>
>> > If rate is the same as set it's correct case.
>> >
>> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> > ---
>> > Based on net-next/master
>> >
>> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
>> > index e4d6edf..dbe9167 100644
>> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
>> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
>> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
>> > return -EINVAL;
>> >
>> > if (ch->rate == rate)
>> > - return rate;
>> > + return 0;
>>
>> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
>> makes sure this can never, ever, happen.
> In current circumstances yes, it will never happen.
> But I caught it while adding related code and better return 0 if upper caller
> doesn't have such check. Suppose that cpdma module is responsible for itself
> and if it's critical I can send this patch along with whole related series.
You have to decide one way or the other, who is responsible.
I think checking higher up is better because it's cheaper at that point to
look at the per-netdev queue rate setting before moving down deeper into the
driver specific data-structures.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpdma: rate is not changed - correct case
From: Ivan Khoronzhuk @ 2017-12-07 19:48 UTC (permalink / raw)
To: David Miller; +Cc: grygorii.strashko, netdev, linux-omap, linux-kernel
In-Reply-To: <20171206.163545.906490902377440615.davem@davemloft.net>
On Wed, Dec 06, 2017 at 04:35:45PM -0500, David Miller wrote:
> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Date: Wed, 6 Dec 2017 16:41:18 +0200
>
> > If rate is the same as set it's correct case.
> >
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> > ---
> > Based on net-next/master
> >
> > drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> > index e4d6edf..dbe9167 100644
> > --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> > @@ -841,7 +841,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
> > return -EINVAL;
> >
> > if (ch->rate == rate)
> > - return rate;
> > + return 0;
>
> Looking at the one and only caller of this function, cpsw_ndo_set_tx_maxrate, it
> makes sure this can never, ever, happen.
In current circumstances yes, it will never happen.
But I caught it while adding related code and better return 0 if upper caller
doesn't have such check. Suppose that cpdma module is responsible for itself
and if it's critical I can send this patch along with whole related series.
>
> So I would instead remove this check completely since it can never trigger.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 06/10] ixgbe: restore offloaded SAs after a reset
From: Shannon Nelson @ 2017-12-07 18:47 UTC (permalink / raw)
To: Alexander Duyck
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <CAKgT0UfDFyH8qjg_-cdGYx5f06iTwydcMTcZ5FvhKJ+=2bNTLQ@mail.gmail.com>
On 12/7/2017 9:16 AM, Alexander Duyck wrote:
> On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> On 12/5/2017 9:30 AM, Alexander Duyck wrote:
>>>
>>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
>>> <shannon.nelson@oracle.com> wrote:
>>>>
>>>> On a chip reset most of the table contents are lost, so must be
<snip>
>>>> + /* reload the IP addrs */
>>>> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
>>>> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i];
>>>> +
>>>> + if (ipsa->used)
>>>> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
>>>> + else
>>>> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf);
>>>
>>>
>>> If we are doing a restore do we actually need to write the zero
>>> values? If we did a reset I thought you had a function that was going
>>> through and zeroing everything out. If so this now becomes redundant.
>>
>>
>> Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It
>> should probably get run at remove as well. Doing this is a bit of safety
>> paranoia, and making sure the CAM memory structures that don't get cleared
>> on reset have exactly what I expect in them.
>
> You might just move ixgbe_ipsec_clear_hw_tables into the rest logic
> itself. Then it covers all cases where you would be resetting the
> hardware and expecting a consistent state. It will mean writing some
> registers twice during the reset but it is probably better just to
> make certain everything stays in a known good state after a reset.
If it is a small number, e.g. 10 or 20, then you may be right. However,
given we have table space for 2k different SAs, at 6 writes per Tx SA
and 10 writes per Rx SA, plus 128 IP address with 4 writes each, we are
already looking at 17K writes already to be sure the tables are clean.
Unfortunately, I don't really know what a "typical" case will be, so I
don't know how many SA we may be offloading at any one time. But in a
busy cloud support server, we might have nearly full tables. If we do
the full clean first, then have to fill all the tables, we're now
looking at up to 35k writes slowing down the reset process.
I'd rather keep it to the constant 17K writes for now, and look later at
using the VALID bit in the IPSRXMOD to see if we can at least cut down
on the Rx writes.
sln
^ permalink raw reply
* [PATH net 4/4] tcp: evaluate packet losses upon RTT change
From: Yuchung Cheng @ 2017-12-07 19:33 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, priyarjha, Yuchung Cheng
In-Reply-To: <20171207193333.59039-1-ycheng@google.com>
RACK skips an ACK unless it advances the most recently delivered
TX timestamp (rack.mstamp). Since RACK also uses the most recent
RTT to decide if a packet is lost, RACK should still run the
loss detection whenever the most recent RTT changes. For example,
an ACK that does not advance the timestamp but triggers the cwnd
undo due to reordering, would then use the most recent (higher)
RTT measurement to detect further losses.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Priyaranjan Jha <priyarjha@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_recovery.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index 0c182303e62e..3a81720ac0c4 100644
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -117,13 +117,8 @@ void tcp_rack_advance(struct tcp_sock *tp, u8 sacked, u32 end_seq,
{
u32 rtt_us;
- if (tp->rack.mstamp &&
- !tcp_rack_sent_after(xmit_time, tp->rack.mstamp,
- end_seq, tp->rack.end_seq))
- return;
-
rtt_us = tcp_stamp_us_delta(tp->tcp_mstamp, xmit_time);
- if (sacked & TCPCB_RETRANS) {
+ if (rtt_us < tcp_min_rtt(tp) && (sacked & TCPCB_RETRANS)) {
/* If the sacked packet was retransmitted, it's ambiguous
* whether the retransmission or the original (or the prior
* retransmission) was sacked.
@@ -134,13 +129,15 @@ void tcp_rack_advance(struct tcp_sock *tp, u8 sacked, u32 end_seq,
* so it's at least one RTT (i.e., retransmission is at least
* an RTT later).
*/
- if (rtt_us < tcp_min_rtt(tp))
- return;
+ return;
}
- tp->rack.rtt_us = rtt_us;
- tp->rack.mstamp = xmit_time;
- tp->rack.end_seq = end_seq;
tp->rack.advanced = 1;
+ tp->rack.rtt_us = rtt_us;
+ if (tcp_rack_sent_after(xmit_time, tp->rack.mstamp,
+ end_seq, tp->rack.end_seq)) {
+ tp->rack.mstamp = xmit_time;
+ tp->rack.end_seq = end_seq;
+ }
}
/* We have waited long enough to accommodate reordering. Mark the expired
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply related
* [PATH net 3/4] tcp: fix off-by-one bug in RACK
From: Yuchung Cheng @ 2017-12-07 19:33 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, priyarjha, Yuchung Cheng
In-Reply-To: <20171207193333.59039-1-ycheng@google.com>
RACK should mark a packet lost when remaining wait time is zero.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Priyaranjan Jha <priyarjha@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_recovery.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index 3143664902e9..0c182303e62e 100644
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -80,12 +80,12 @@ static void tcp_rack_detect_loss(struct sock *sk, u32 *reo_timeout)
*/
remaining = tp->rack.rtt_us + reo_wnd -
tcp_stamp_us_delta(tp->tcp_mstamp, skb->skb_mstamp);
- if (remaining < 0) {
+ if (remaining <= 0) {
tcp_rack_mark_skb_lost(sk, skb);
list_del_init(&skb->tcp_tsorted_anchor);
} else {
- /* Record maximum wait time (+1 to avoid 0) */
- *reo_timeout = max_t(u32, *reo_timeout, 1 + remaining);
+ /* Record maximum wait time */
+ *reo_timeout = max_t(u32, *reo_timeout, remaining);
}
}
}
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply related
* [PATH net 2/4] tcp: always evaluate losses in RACK upon undo
From: Yuchung Cheng @ 2017-12-07 19:33 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, priyarjha, Yuchung Cheng
In-Reply-To: <20171207193333.59039-1-ycheng@google.com>
When sender detects spurious retransmission, all packets
marked lost are remarked to be in-flight. However some may
be considered lost based on its timestamps in RACK. This patch
forces RACK to re-evaluate, which may be skipped previously if
the ACK does not advance RACK timestamp.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Priyaranjan Jha <priyarjha@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 734cfc8ff76e..5a240f9d208b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2326,6 +2326,7 @@ static void tcp_undo_cwnd_reduction(struct sock *sk, bool unmark_loss)
}
tp->snd_cwnd_stamp = tcp_jiffies32;
tp->undo_marker = 0;
+ tp->rack.advanced = 1; /* Force RACK to re-exam losses */
}
static inline bool tcp_may_undo(const struct tcp_sock *tp)
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply related
* [PATH net 1/4] tcp: correctly test congestion state in RACK
From: Yuchung Cheng @ 2017-12-07 19:33 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, priyarjha, Yuchung Cheng
In-Reply-To: <20171207193333.59039-1-ycheng@google.com>
RACK does not test the loss recovery state correctly to compute
the reordering window. It assumes if lost_out is zero then TCP is
not in loss recovery. But it can be zero during recovery before
calling tcp_rack_detect_loss(): when an ACK acknowledges all
packets marked lost before receiving this ACK, but has not yet
to discover new ones by tcp_rack_detect_loss(). The fix is to
simply test the congestion state directly.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Priyaranjan Jha <priyarjha@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_recovery.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index d3ea89020c69..3143664902e9 100644
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -55,7 +55,8 @@ static void tcp_rack_detect_loss(struct sock *sk, u32 *reo_timeout)
* to queuing or delayed ACKs.
*/
reo_wnd = 1000;
- if ((tp->rack.reord || !tp->lost_out) && min_rtt != ~0U) {
+ if ((tp->rack.reord || inet_csk(sk)->icsk_ca_state < TCP_CA_Recovery) &&
+ min_rtt != ~0U) {
reo_wnd = max((min_rtt >> 2) * tp->rack.reo_wnd_steps, reo_wnd);
reo_wnd = min(reo_wnd, tp->srtt_us >> 3);
}
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply related
* [PATH net 0/4] tcp: RACK loss recovery bug fixes
From: Yuchung Cheng @ 2017-12-07 19:33 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, priyarjha, Yuchung Cheng
This patch set has four minor bug fixes in TCP RACK loss recovery.
Yuchung Cheng (4):
tcp: correctly test congestion state in RACK
tcp: always evaluate losses in RACK upon undo
tcp: fix off-by-one bug in RACK
tcp: evaluate packet losses upon RTT change
net/ipv4/tcp_input.c | 1 +
net/ipv4/tcp_recovery.c | 28 +++++++++++++---------------
2 files changed, 14 insertions(+), 15 deletions(-)
--
2.15.1.424.g9478a66081-goog
^ permalink raw reply
* Re: [PATCH] usbnet: fix alignment for frames with no ethernet header
From: David Miller @ 2017-12-07 19:33 UTC (permalink / raw)
To: bjorn; +Cc: netdev, jay, linux-usb, oneukum
In-Reply-To: <87y3mepdkz.fsf@miraculix.mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 07 Dec 2017 20:01:16 +0100
> David Miller <davem@davemloft.net> writes:
>
>> From: Bjørn Mork <bjorn@mork.no>
>> Date: Wed, 6 Dec 2017 20:21:24 +0100
>>
>>> The qmi_wwan minidriver support a 'raw-ip' mode where frames are
>>> received without any ethernet header. This causes alignment issues
>>> because the skbs allocated by usbnet are "IP aligned".
>>>
>>> Fix by allowing minidrivers to disable the additional alignment
>>> offset. This is implemented using a per-device flag, since the same
>>> minidriver also supports 'ethernet' mode.
>>>
>>> Fixes: 32f7adf633b9 ("net: qmi_wwan: support "raw IP" mode")
>>> Reported-and-tested-by: Jay Foster <jay@systech.com>
>>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>>
>> Looks good, applied and queued up for -stable.
>
>
> Thanks. I can see it in the -stable queue, but it didn't show up here
> yet: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>
> Did it get stuck somewhere?
Caught again :-)
I do GIT work on two different machines (one at home and one at the
office) and sometimes I head to the office before pushing everything
out on the machine at home :-/
I've pushed this specific patch out to 'net'.
^ permalink raw reply
* Re: [ovs-dev] [PATCH 7/8] net: ovs: remove unused hardirq.h
From: Pravin Shelar @ 2017-12-07 19:27 UTC (permalink / raw)
To: Yang Shi
Cc: linux-kernel, ovs dev, Linux Kernel Network Developers, linux-mm,
Pravin Shelar, linux-crypto, linux-fsdevel, David S. Miller
In-Reply-To: <1510959741-31109-7-git-send-email-yang.s@alibaba-inc.com>
On Fri, Nov 17, 2017 at 3:02 PM, Yang Shi <yang.s@alibaba-inc.com> wrote:
> Preempt counter APIs have been split out, currently, hardirq.h just
> includes irq_enter/exit APIs which are not used by openvswitch at all.
>
> So, remove the unused hardirq.h.
>
> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> Cc: Pravin Shelar <pshelar@nicira.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: dev@openvswitch.org
Acked-by: Pravin B Shelar <pshelar@ovn.org>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] selftests: net: Adding config fragment CONFIG_CGROUP_BPF=y
From: Naresh Kamboju @ 2017-12-07 19:23 UTC (permalink / raw)
To: netdev; +Cc: shuahkh, guro, shuah, linux-kselftest
CONFIG_CGROUP_BPF=y is required for test_dev_cgroup test case.
Signed-off-by: Naresh Kamboju <naresh.kamboju@linaro.org>
---
tools/testing/selftests/net/config | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index e57b4ac..02301c6 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -1,3 +1,4 @@
CONFIG_USER_NS=y
CONFIG_BPF_SYSCALL=y
CONFIG_TEST_BPF=m
+CONFIG_CGROUP_BPF=y
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net 2/4] net: aquantia: Fix hardware DMA stream overload on large MRRS
From: David Miller @ 2017-12-07 19:23 UTC (permalink / raw)
To: igor.russkikh
Cc: netdev, darcari, pavel.belous, Nadezhda.Krupnina, simon.edelhaus
In-Reply-To: <5e0faf4821c3d01d1de296da18f6c07877223c4f.1512548097.git.igor.russkikh@aquantia.com>
From: Igor Russkikh <igor.russkikh@aquantia.com>
Date: Thu, 7 Dec 2017 11:39:43 +0300
> @@ -2343,6 +2343,9 @@
> #define tx_dma_desc_base_addrmsw_adr(descriptor) \
> (0x00007c04u + (descriptor) * 0x40)
>
> +/* tx dma total request limit */
> +#define tx_dma_total_req_limit_adr 0x00007b20u
> +
> /* tx interrupt moderation control register definitions
> * Preprocessor definitions for TX Interrupt Moderation Control Register
> * Base Address: 0x00008980
> @@ -2369,6 +2372,9 @@
> /* default value of bitfield reg_res_dsbl */
> #define pci_reg_res_dsbl_default 0x1
>
> +/* PCI core control register */
> +#define pci_reg_control6_adr 0x1014u
> +
I should have given you this feedback a long time ago, but better late
than never...
CPP macros, especially those which define register numbers or bit
valus, should never use lowercase. They should always use uppercase
names.
This is a coding style convention we use in the entire kernel which
makes it easy to visually see whether something is a bonafide C
symbol or a CPP macro.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
From: David Ahern @ 2017-12-07 19:22 UTC (permalink / raw)
To: Roman Gushchin, netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
Quentin Monnet
In-Reply-To: <20171207183909.16240-5-guro@fb.com>
On 12/7/17 11:39 AM, Roman Gushchin wrote:
> This patch adds basic cgroup bpf operations to bpftool:
> cgroup list, attach and detach commands.
>
> Usage is described in the corresponding man pages,
> and examples are provided.
>
> Syntax:
> $ bpftool cgroup list CGROUP
> $ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]
> $ bpftool cgroup detach CGROUP ATTACH_TYPE PROG
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
LGTM.
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH 4/8] vfs: remove unused hardirq.h
From: Al Viro @ 2017-12-07 19:21 UTC (permalink / raw)
To: Yang Shi; +Cc: linux-kernel, linux-mm, linux-fsdevel, linux-crypto, netdev
In-Reply-To: <0bfadf85-b499-5d2f-f0d2-20d229ba7fe2@alibaba-inc.com>
On Fri, Dec 08, 2017 at 03:12:52AM +0800, Yang Shi wrote:
> Hi folks,
>
> Any comment on this one?
Applied
^ permalink raw reply
* RE: [PATCH 8/8] net: tipc: remove unused hardirq.h
From: Jon Maloy @ 2017-12-07 19:20 UTC (permalink / raw)
To: Yang Shi, linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
linux-crypto@vger.kernel.org, netdev@vger.kernel.org, Ying Xue,
David S. Miller
In-Reply-To: <da42d136-4e51-6d04-4120-cb53df03c661@alibaba-inc.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Yang Shi
> Sent: Thursday, December 07, 2017 14:16
> To: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org; linux-fsdevel@vger.kernel.org; linux-
> crypto@vger.kernel.org; netdev@vger.kernel.org; Jon Maloy
> <jon.maloy@ericsson.com>; Ying Xue <ying.xue@windriver.com>; David S.
> Miller <davem@davemloft.net>
> Subject: Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
>
> Hi folks,
>
> Any comment on this one?
If it compiles it is ok with me. Don't know why it was put there in the first place.
///jon
>
> Thanks,
> Yang
>
>
> On 11/17/17 3:02 PM, Yang Shi wrote:
> > Preempt counter APIs have been split out, currently, hardirq.h just
> > includes irq_enter/exit APIs which are not used by TIPC at all.
> >
> > So, remove the unused hardirq.h.
> >
> > Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> > Cc: Jon Maloy <jon.maloy@ericsson.com>
> > Cc: Ying Xue <ying.xue@windriver.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > ---
> > net/tipc/core.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/net/tipc/core.h b/net/tipc/core.h index 5cc5398..099e072
> > 100644
> > --- a/net/tipc/core.h
> > +++ b/net/tipc/core.h
> > @@ -49,7 +49,6 @@
> > #include <linux/uaccess.h>
> > #include <linux/interrupt.h>
> > #include <linux/atomic.h>
> > -#include <asm/hardirq.h>
> > #include <linux/netdevice.h>
> > #include <linux/in.h>
> > #include <linux/list.h>
> >
^ permalink raw reply
* Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
From: Yang Shi @ 2017-12-07 19:15 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linux-fsdevel, linux-crypto, netdev, Jon Maloy,
Ying Xue, David S. Miller
In-Reply-To: <1510959741-31109-8-git-send-email-yang.s@alibaba-inc.com>
Hi folks,
Any comment on this one?
Thanks,
Yang
On 11/17/17 3:02 PM, Yang Shi wrote:
> Preempt counter APIs have been split out, currently, hardirq.h just
> includes irq_enter/exit APIs which are not used by TIPC at all.
>
> So, remove the unused hardirq.h.
>
> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
> net/tipc/core.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/tipc/core.h b/net/tipc/core.h
> index 5cc5398..099e072 100644
> --- a/net/tipc/core.h
> +++ b/net/tipc/core.h
> @@ -49,7 +49,6 @@
> #include <linux/uaccess.h>
> #include <linux/interrupt.h>
> #include <linux/atomic.h>
> -#include <asm/hardirq.h>
> #include <linux/netdevice.h>
> #include <linux/in.h>
> #include <linux/list.h>
>
^ permalink raw reply
* Re: [PATCH 7/8] net: ovs: remove unused hardirq.h
From: Yang Shi @ 2017-12-07 19:14 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, David S. Miller
In-Reply-To: <7877c3bd-74ec-a7d2-61d3-85a4c452e710-gPhfCIXyaqCqndwCJWfcng@public.gmane.org>
Hi folks,
Any comment on this one?
Thanks,
Yang
On 11/17/17 5:48 PM, Yang Shi wrote:
> It looks the email address of Pravin in MAINTAINERS file is obsolete,
> sent to the right address.
>
> Yang
>
>
> On 11/17/17 3:02 PM, Yang Shi wrote:
>> Preempt counter APIs have been split out, currently, hardirq.h just
>> includes irq_enter/exit APIs which are not used by openvswitch at all.
>>
>> So, remove the unused hardirq.h.
>>
>> Signed-off-by: Yang Shi <yang.s-gPhfCIXyaqCqndwCJWfcng@public.gmane.org>
>> Cc: Pravin Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
>> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>> Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org
>> ---
>> net/openvswitch/vport-internal_dev.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/net/openvswitch/vport-internal_dev.c
>> b/net/openvswitch/vport-internal_dev.c
>> index 04a3128..2f47c65 100644
>> --- a/net/openvswitch/vport-internal_dev.c
>> +++ b/net/openvswitch/vport-internal_dev.c
>> @@ -16,7 +16,6 @@
>> * 02110-1301, USA
>> */
>> -#include <linux/hardirq.h>
>> #include <linux/if_vlan.h>
>> #include <linux/kernel.h>
>> #include <linux/netdevice.h>
>>
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] net: thunderx: add support for PTP clock
From: David Miller @ 2017-12-07 19:14 UTC (permalink / raw)
To: aleksey.makarov
Cc: netdev, linux-arm-kernel, linux-kernel, Sunil.Goutham, rad, rric,
ddaney
In-Reply-To: <20171207.133932.904724095727636711.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 07 Dec 2017 13:39:32 -0500 (EST)
> From: Aleksey Makarov <aleksey.makarov@cavium.com>
> Date: Wed, 6 Dec 2017 16:30:56 +0300
>
>> This series adds support for IEEE 1588 Precision Time Protocol
>> to Cavium ethernet driver.
>
> Series applied, thank you.
Actually, this doesn't even compile, I'm reverting:
drivers/net/ethernet/cavium/common/cavium_ptp.c:64:20: error: redefinition of ‘cavium_ptp_get’
struct cavium_ptp *cavium_ptp_get(void)
^~~~~~~~~~~~~~
In file included from drivers/net/ethernet/cavium/common/cavium_ptp.c:19:0:
drivers/net/ethernet/cavium/common/cavium_ptp.h:59:34: note: previous definition of ‘cavium_ptp_get’ was here
static inline struct cavium_ptp *cavium_ptp_get(void)
^~~~~~~~~~~~~~
drivers/net/ethernet/cavium/common/cavium_ptp.c:84:6: error: redefinition of ‘cavium_ptp_put’
void cavium_ptp_put(struct cavium_ptp *ptp)
^~~~~~~~~~~~~~
In file included from drivers/net/ethernet/cavium/common/cavium_ptp.c:19:0:
drivers/net/ethernet/cavium/common/cavium_ptp.h:64:20: note: previous definition of ‘cavium_ptp_put’ was here
static inline void cavium_ptp_put(struct cavium_ptp *ptp) {}
^~~~~~~~~~~~~~
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox