* Re: [PATCH] net: ethernet: stmmac: add ARP management
From: Florian Fainelli @ 2017-01-17 17:41 UTC (permalink / raw)
To: Andrew Lunn, Christophe Roullier
Cc: Alexandre Torgue, Giuseppe Cavallaro, netdev, linux-kernel
In-Reply-To: <20170117172541.GC9470@lunn.ch>
On 01/17/2017 09:25 AM, Andrew Lunn wrote:
> On Tue, Jan 17, 2017 at 05:56:40PM +0100, Christophe Roullier wrote:
>> DWC_ether_qos supports the Address Recognition
>
> Resolution not Recognition?
>
>> Protocol (ARP) Offload for IPv4 packets. This feature
>> allows the processing of the IPv4 ARP request packet
>> in the receive path and generating corresponding ARP
>> response packet in the transmit path. DWC_ether_qos
>> generates the ARP reply packets for appropriate ARP
>> request packets.
>
> What about when .ndo_set_mac_address is called?
Was wondering about the same thing, but presumably, if there is correct
programming of the MAC address into the appropriate MAC address
registers, one could expect the hardware to latch that value while
generating the ARP replies?
While it sounds like this feature may be useful under heavy ARP spoofing
workloads, in practice, ARP packets are both small and infrequent, so
this sounds like micro optimization.
--
Florian
^ permalink raw reply
* [net, 6/6] net: korina: version bump
From: Roman Yeryomin @ 2017-01-17 17:33 UTC (permalink / raw)
To: netdev
Signed-off-by: Roman Yeryomin <roman@advem.lv>
---
drivers/net/ethernet/korina.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index 83c994f..c8fed01 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -66,8 +66,8 @@
#include <asm/mach-rc32434/dma_v.h>
#define DRV_NAME "korina"
-#define DRV_VERSION "0.10"
-#define DRV_RELDATE "04Mar2008"
+#define DRV_VERSION "0.20"
+#define DRV_RELDATE "15Jan2017"
#define STATION_ADDRESS_HIGH(dev) (((dev)->dev_addr[0] << 8) | \
((dev)->dev_addr[1]))
--
2.7.4
^ permalink raw reply related
* Re: Kernel 4.6.7-rt14 kernel workqueue lockup - rtnl deadlock plus syscall endless loop
From: David Miller @ 2017-01-17 17:57 UTC (permalink / raw)
To: EladN; +Cc: netdev
In-Reply-To: <AM5PR0701MB187338B2418A19E1B4B55C98A07C0@AM5PR0701MB1873.eurprd07.prod.outlook.com>
From: Elad Nachman <EladN@gilat.com>
Date: Tue, 17 Jan 2017 17:39:03 +0000
> What's more odd about this is that it's very unusual and strange for
> a kernel function to invoke the restart mechanism because a lock is
> being held - the point of the restart mechanism is to allow userspace
> signal handlers to run, so it should only be used when there's a
> signal pending. I think this is a hack in the IPv6 code to work
> around some other issue.
It's not unusal at all, if you actually grep for this under net/ you will
see that it is in fact a common code pattern.
It prevents deadlocks because the sysfs and other nodes that we are
operating with can be unregistered by other threads of control holding
the RTNL mutex. If we don't break out, we won't release our reference
and therefore the RTNL mutex holding entity cannot make forward
progress.
This behavior is therefore very much intentional.
^ permalink raw reply
* Re: [net, 3/6] net: korina: increase tx/rx ring sizes
From: David Miller @ 2017-01-17 17:58 UTC (permalink / raw)
To: leroi.lists; +Cc: netdev
In-Reply-To: <1484674356-22363-1-git-send-email-roman@advem.lv>
From: Roman Yeryomin <leroi.lists@gmail.com>
Date: Tue, 17 Jan 2017 19:32:36 +0200
> Having larger ring sizes almost eliminates rx fifo overflow, thus improving performance.
> This patch reduces rx overflow occurence by approximately 1000 times (from ~25k down to ~25 times per 3M frames)
Those numbers don't mean much without full context.
What kind of system, what kind of traffic, and over what kind of link?
^ permalink raw reply
* Re: Kernel 4.6.7-rt14 kernel workqueue lockup - rtnl deadlock plus syscall endless loop
From: Stephen Hemminger @ 2017-01-17 17:53 UTC (permalink / raw)
To: Elad Nachman; +Cc: netdev@vger.kernel.org
In-Reply-To: <AM5PR0701MB187338B2418A19E1B4B55C98A07C0@AM5PR0701MB1873.eurprd07.prod.outlook.com>
On Tue, 17 Jan 2017 17:39:03 +0000
Elad Nachman <EladN@gilat.com> wrote:
> Hi,
>
> I am experiencing sporadic work queue lockups on kernel 4.6.7-rt14 (mach-socfpga).
>
> Using a HW debugger I got the following information:
>
> A process containing a network namespace is terminating itself (SIGKILL), which causes cleanup_net() to be scheduled to kworker/u4:2 to clean up the network namespace running on the process.
>
> Kworker/u4:2 got preempted (plus there are a lot of other work queue items, like vmstat_shepherd, wakeup_dirtytime_writeback, phy_state_machine, neigh_periodic_work, check_lifetime plus another one by a LKM) while holding the rtnl lock.
>
> A processing running waitpid() on the terminated process starts a new process, which forks busybox to run sysctl -w net.ipv6.conf.all.forwarding = 1 .
> This in turn starts making a write syscall, calling in turn vfs_write, proc_sys_call_handler, addrconf_sysctl_forward, and finally addrconf_fixup_forwarding().
>
> addrconf_fixup_forwarding() runs the following code:
>
> if (!rtnl_trylock())
> return restart_syscall();
>
> This fails and restart_syscall() does the following:
>
> set_tsk_thread_flag(current, TIF_SIGPENDING);
> return -ERESTARTNOINTR;
>
> Now the system call goes back to ret_fast_syscall (arch/arm/kernel/entry-common.S)
> Testing the flags in the task_struct (which contain TIF_SIGPENDING) the code branches to fast_work_pending, then falls through to slow_work_pending, which
> Calls do_work_pending(), and in turn calls do_signal(), get_signal(), dequeuer_signal(), which find no signals, and clears the TIF_SIGPENDING bit when recalc_sigpending() is called, then returns zero.
>
> This causes do_signal() to examine r0 and return 1 (-ERESTARTNOINTR), which is propogated to the assembly code by do_work_pending().
> Having r0 equal zero causes a branch to local_restart, which restarts the very same write system call in an endless loop.
> No scheduling is possible, so the cleanup_net() cannot finish and release rtnl, which in turn causes the endless restarting of the write system call.
>
> I have sent this to linux-arm-kernel and got a response from Russel King saying that (relating to addrconf_fixup_forwarding, net/ipv6/addrconf.c ):
>
> "
> I think the problem is that:
>
> if (!rtnl_trylock())
> return restart_syscall();
>
>
>
> which, if it didn't do a trylock, it would put this thread to sleep
> and allow other threads to run (potentially allowing the holder of
> the lock to release it.)
>
> What's more odd about this is that it's very unusual and strange for
> a kernel function to invoke the restart mechanism because a lock is
> being held - the point of the restart mechanism is to allow userspace
> signal handlers to run, so it should only be used when there's a
> signal pending. I think this is a hack in the IPv6 code to work
> around some other issue.
The trylock was added intentionally to handle a different deadlock.
Going back to a blocking lock would cause that problem.
There was a deadlock between device unregistration and sysfs access.
Unregistration wants to remove sysfs entry while holding RTNL.
Sysfs access graps sysfs file entry lock then acquires RTNL.
The patch back in 2.6.30 followed by multiple revisions was to
restart the sysfs write syscall.
^ permalink raw reply
* Re: [PATCH net] lwtunnel: fix autoload of lwt modules
From: David Ahern @ 2017-01-17 18:04 UTC (permalink / raw)
To: Robert Shearman, netdev, roopa
In-Reply-To: <0e226af4-cb62-9b20-f8a2-191b0aaffb11@brocade.com>
On 1/17/17 10:26 AM, Robert Shearman wrote:
> Ok, I'll continue looking too and let you know if there's anything else that pops up.
>
> Having said that, even if we eliminate all the unreferenced objects in the current code, what are the chances that we'll be able to keep it this way going forward? Perhaps it would be safer to retry the insert operation from as close to the start as possible?
handling restart for all code paths seems a bit risky for 4.10. Perhaps then the best course for 4.10 and older stable releases is to remove the autoload code from lwtunnel. It can be re-added once the recovery path is handled.
^ permalink raw reply
* [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
From: Rolf Neugebauer @ 2017-01-17 18:13 UTC (permalink / raw)
To: netdev; +Cc: mst, davem, jasowang, rppt, Rolf Neugebauer
This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
subtle change in how the virtio_net flags are derived from the SKBs
ip_summed field.
With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
ip_summed == CHECKSUM_NONE, which should be the same.
Further, the virtio spec 1.0 / CS04 explicitly says that
VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
---
include/linux/virtio_net.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 66204007d7ac..56436472ccc7 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
skb_checksum_start_offset(skb));
hdr->csum_offset = __cpu_to_virtio16(little_endian,
skb->csum_offset);
- } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
- hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
} /* else everything is zero */
return 0;
--
2.11.0
^ permalink raw reply related
* RE: Kernel 4.6.7-rt14 kernel workqueue lockup - rtnl deadlock plus syscall endless loop
From: Elad Nachman @ 2017-01-17 18:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20170117.125748.122599514616047216.davem@davemloft.net>
Any thought about limiting the amount of busy polling?
Say if more than X polls are done within a jiffy, then at least for preemptable kernels you can sleep for a jiffy inside the syscall to yield the CPU for a while?
Thanks,
Elad.
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: יום ג 17 ינואר 2017 19:58
To: Elad Nachman <EladN@gilat.com>
Cc: netdev@vger.kernel.org
Subject: Re: Kernel 4.6.7-rt14 kernel workqueue lockup - rtnl deadlock plus syscall endless loop
From: Elad Nachman <EladN@gilat.com>
Date: Tue, 17 Jan 2017 17:39:03 +0000
> What's more odd about this is that it's very unusual and strange for a
> kernel function to invoke the restart mechanism because a lock is
> being held - the point of the restart mechanism is to allow userspace
> signal handlers to run, so it should only be used when there's a
> signal pending. I think this is a hack in the IPv6 code to work around
> some other issue.
It's not unusal at all, if you actually grep for this under net/ you will see that it is in fact a common code pattern.
It prevents deadlocks because the sysfs and other nodes that we are operating with can be unregistered by other threads of control holding the RTNL mutex. If we don't break out, we won't release our reference and therefore the RTNL mutex holding entity cannot make forward progress.
This behavior is therefore very much intentional.
IMPORTANT - This email and any attachments is intended for the above named addressee(s), and may contain information which is confidential or privileged. If you are not the intended recipient, please inform the sender immediately and delete this email: you should not copy or use this e-mail for any purpose nor disclose its contents to any person.
^ permalink raw reply
* Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c
From: Alexei Starovoitov @ 2017-01-17 18:16 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, Kernel Team
In-Reply-To: <587DDED4.5060706@iogearbox.net>
On 1/17/17 1:07 AM, Daniel Borkmann wrote:
> On 01/17/2017 07:17 AM, Martin KaFai Lau wrote:
>> test_lru_sanity5() fails when the number of online cpus
>> is fewer than the number of possible cpus. It can be
>> reproduced with qemu by using cmd args "--smp cpus=2,maxcpus=8".
>>
>> The problem is the loop in test_lru_sanity5() is testing
>> 'i' which is incorrect.
>>
>> This patch:
>> 1. Make sched_next_online() always return -1 if it cannot
>> find a next cpu to schedule the process.
>> 2. In test_lru_sanity5(), the parent process does
>> sched_setaffinity() first (through sched_next_online())
>> and the forked process will inherit it according to
>> the 'man sched_setaffinity'.
>>
>> Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
>> Reported-by: Daniel Borkmann <daniel@iogearbox.net>
>> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
>
> Looks good, thanks for fixing!
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>
> (Patch is against -net tree.)
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net 1/1] net sched actions: fix refcnt when GETing of action after bind
From: Cong Wang @ 2017-01-17 18:17 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David Miller, Linux Kernel Network Developers
In-Reply-To: <1484493246-10911-1-git-send-email-jhs@emojatatu.com>
On Sun, Jan 15, 2017 at 7:14 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 2095c83..e10456ef6f 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -900,8 +900,6 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
> goto err;
> }
> act->order = i;
> - if (event == RTM_GETACTION)
> - act->tcfa_refcnt++;
> list_add_tail(&act->list, &actions);
> }
>
> @@ -914,7 +912,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
> return ret;
> }
> err:
> - tcf_action_destroy(&actions, 0);
> + if (event != RTM_GETACTION)
> + tcf_action_destroy(&actions, 0);
Why this check for RTM_GETACTION? It does not make sense
at least for the error case, that is, when tcf_action_get_1() fails
in the middle of the loop, all the previous ones should be destroyed
no matter it's GETACTION or DELACTION...
Also, for the non-error case of GETACTION, they should be
destroyed too after dumping to user-space, otherwise there is no
way to use them since 'actions' is local to that function.
I thought in commit aecc5cefc389 you grab that refcnt on purpose.
^ permalink raw reply
* Re: [net, 2/6] net: korina: define NAPI_WEIGHT
From: Roman Yeryomin @ 2017-01-17 18:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170117.124006.1559010522356313497.davem@davemloft.net>
On 17 January 2017 at 19:40, David Miller <davem@davemloft.net> wrote:
> From: Roman Yeryomin <leroi.lists@gmail.com>
> Date: Tue, 17 Jan 2017 19:32:22 +0200
>
>> @@ -77,6 +77,8 @@
>>
>> #define MII_CLOCK 1250000 /* no more than 2.5MHz */
>>
>> +#define NAPI_WEIGHT 64
>> +
>> /* the following must be powers of two */
>> #define KORINA_NUM_RDS 64 /* number of receive descriptors */
>> #define KORINA_NUM_TDS 64 /* number of transmit descriptors */
>> @@ -1080,7 +1082,7 @@ static int korina_probe(struct platform_device *pdev)
>> dev->netdev_ops = &korina_netdev_ops;
>> dev->ethtool_ops = &netdev_ethtool_ops;
>> dev->watchdog_timeo = TX_TIMEOUT;
>> - netif_napi_add(dev, &lp->napi, korina_poll, 64);
>> + netif_napi_add(dev, &lp->napi, korina_poll, NAPI_WEIGHT);
>
> Please use NAPI_POLL_WEIGHT from linux/netdevice.h
OK, should I resend the whole set or just this one and dependant?
Regards,
Roman
^ permalink raw reply
* Re: [net, 2/6] net: korina: define NAPI_WEIGHT
From: Florian Fainelli @ 2017-01-17 18:32 UTC (permalink / raw)
To: Roman Yeryomin, David Miller; +Cc: netdev
In-Reply-To: <CACiydbLyswWZ2ZX04m22Ga_FBbp-52kxOxBbNcvS+R1d1Kdpcw@mail.gmail.com>
On 01/17/2017 10:30 AM, Roman Yeryomin wrote:
> On 17 January 2017 at 19:40, David Miller <davem@davemloft.net> wrote:
>> From: Roman Yeryomin <leroi.lists@gmail.com>
>> Date: Tue, 17 Jan 2017 19:32:22 +0200
>>
>>> @@ -77,6 +77,8 @@
>>>
>>> #define MII_CLOCK 1250000 /* no more than 2.5MHz */
>>>
>>> +#define NAPI_WEIGHT 64
>>> +
>>> /* the following must be powers of two */
>>> #define KORINA_NUM_RDS 64 /* number of receive descriptors */
>>> #define KORINA_NUM_TDS 64 /* number of transmit descriptors */
>>> @@ -1080,7 +1082,7 @@ static int korina_probe(struct platform_device *pdev)
>>> dev->netdev_ops = &korina_netdev_ops;
>>> dev->ethtool_ops = &netdev_ethtool_ops;
>>> dev->watchdog_timeo = TX_TIMEOUT;
>>> - netif_napi_add(dev, &lp->napi, korina_poll, 64);
>>> + netif_napi_add(dev, &lp->napi, korina_poll, NAPI_WEIGHT);
>>
>> Please use NAPI_POLL_WEIGHT from linux/netdevice.h
>
> OK, should I resend the whole set or just this one and dependant?
You need to resend everything and please also provide a cover letter,
and a proper patch subject (e.g: git format-patch --cover-letter
--subject="PATCH net-next v2" a..b)
Thanks!
--
Florian
^ permalink raw reply
* Re: [net, 3/6] net: korina: increase tx/rx ring sizes
From: Roman Yeryomin @ 2017-01-17 18:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170117.125844.253301507252169868.davem@davemloft.net>
On 17 January 2017 at 19:58, David Miller <davem@davemloft.net> wrote:
> From: Roman Yeryomin <leroi.lists@gmail.com>
> Date: Tue, 17 Jan 2017 19:32:36 +0200
>
>> Having larger ring sizes almost eliminates rx fifo overflow, thus improving performance.
>> This patch reduces rx overflow occurence by approximately 1000 times (from ~25k down to ~25 times per 3M frames)
>
> Those numbers don't mean much without full context.
>
> What kind of system, what kind of traffic, and over what kind of link?
MIPS rb532 board, TCP iperf3 test over 100M link, NATed speed ~55Mbps.
I can do more tests and provide more precise numbers, if needed.
Regards,
Roman
^ permalink raw reply
* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
From: Michael S. Tsirkin @ 2017-01-17 18:27 UTC (permalink / raw)
To: Rolf Neugebauer; +Cc: netdev, davem, jasowang, rppt
In-Reply-To: <20170117181351.98291-1-rolf.neugebauer@docker.com>
On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
> subtle change in how the virtio_net flags are derived from the SKBs
> ip_summed field.
>
> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
> ip_summed == CHECKSUM_NONE, which should be the same.
>
> Further, the virtio spec 1.0 / CS04 explicitly says that
> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>
> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Should be backported into stable as well.
> ---
> include/linux/virtio_net.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 66204007d7ac..56436472ccc7 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -91,8 +91,6 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> skb_checksum_start_offset(skb));
> hdr->csum_offset = __cpu_to_virtio16(little_endian,
> skb->csum_offset);
> - } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
> - hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
>
> return 0;
> --
> 2.11.0
^ permalink raw reply
* [PATCH net-next] tcp: accept RST for rcv_nxt - 1 after receiving a FIN
From: Jason Baron @ 2017-01-17 18:37 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Dumazet
From: Jason Baron <jbaron@akamai.com>
Using a Mac OSX box as a client connecting to a Linux server, we have found
that when certain applications (such as 'ab'), are abruptly terminated
(via ^C), a FIN is sent followed by a RST packet on tcp connections. The
FIN is accepted by the Linux stack but the RST is sent with the same
sequence number as the FIN, and Linux responds with a challenge ACK per
RFC 5961. The OSX client then sometimes (they are rate-limited) does not
reply with any RST as would be expected on a closed socket.
This results in sockets accumulating on the Linux server left mostly in
the CLOSE_WAIT state, although LAST_ACK and CLOSING are also possible.
This sequence of events can tie up a lot of resources on the Linux server
since there may be a lot of data in write buffers at the time of the RST.
Accepting a RST equal to rcv_nxt - 1, after we have already successfully
processed a FIN, has made a significant difference for us in practice, by
freeing up unneeded resources in a more expedient fashion.
A packetdrill test demonstrating the behavior:
// testing mac osx rst behavior
// Establish a connection
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
0.000 bind(3, ..., ...) = 0
0.000 listen(3, 1) = 0
0.100 < S 0:0(0) win 32768 <mss 1460,nop,wscale 10>
0.100 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 5>
0.200 < . 1:1(0) ack 1 win 32768
0.200 accept(3, ..., ...) = 4
// Client closes the connection
0.300 < F. 1:1(0) ack 1 win 32768
// now send rst with same sequence
0.300 < R. 1:1(0) ack 1 win 32768
// make sure we are in TCP_CLOSE
0.400 %{
assert tcpi_state == 7
}%
Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_input.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 1a34e9278c07..bfa165cc455a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5199,6 +5199,23 @@ static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
return err;
}
+/* Accept RST for rcv_nxt - 1 after a FIN.
+ * When tcp connections are abruptly terminated from Mac OSX (via ^C), a
+ * FIN is sent followed by a RST packet. The RST is sent with the same
+ * sequence number as the FIN, and thus according to RFC 5961 a challenge
+ * ACK should be sent. However, Mac OSX rate limits replies to challenge
+ * ACKs on the closed socket. In addition middleboxes can drop either the
+ * challenge ACK or a subsequent RST.
+ */
+static bool tcp_reset_check(const struct sock *sk, const struct sk_buff *skb)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ return unlikely(TCP_SKB_CB(skb)->seq == (tp->rcv_nxt - 1) &&
+ (1 << sk->sk_state) & (TCPF_CLOSE_WAIT | TCPF_LAST_ACK |
+ TCPF_CLOSING));
+}
+
/* Does PAWS and seqno based validation of an incoming segment, flags will
* play significant role here.
*/
@@ -5237,20 +5254,25 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
LINUX_MIB_TCPACKSKIPPEDSEQ,
&tp->last_oow_ack_time))
tcp_send_dupack(sk, skb);
+ } else if (tcp_reset_check(sk, skb)) {
+ tcp_reset(sk);
}
goto discard;
}
/* Step 2: check RST bit */
if (th->rst) {
- /* RFC 5961 3.2 (extend to match against SACK too if available):
- * If seq num matches RCV.NXT or the right-most SACK block,
+ /* RFC 5961 3.2 (extend to match against (RCV.NXT - 1) after a
+ * FIN and SACK too if available):
+ * If seq num matches RCV.NXT or (RCV.NXT - 1) after a FIN, or
+ * the right-most SACK block,
* then
* RESET the connection
* else
* Send a challenge ACK
*/
- if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
+ if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt ||
+ tcp_reset_check(sk, skb)) {
rst_seq_match = true;
} else if (tcp_is_sack(tp) && tp->rx_opt.num_sacks > 0) {
struct tcp_sack_block *sp = &tp->selective_acks[0];
--
2.6.1
^ permalink raw reply related
* Re: [PATCH net-next v4 1/2] net sched actions: Add support for user cookies
From: Cong Wang @ 2017-01-17 18:40 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Linux Kernel Network Developers, Jiri Pirko, paulb,
John Fastabend, Simon Horman, Roman Mashak, Hadar Har-Zion,
Or Gerlitz, Roi Dayan, Daniel Borkmann
In-Reply-To: <1484651509-27500-2-git-send-email-jhs@emojatatu.com>
On Tue, Jan 17, 2017 at 3:11 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> @@ -575,6 +583,23 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
> if (err < 0)
> goto err_mod;
>
> + if (tb[TCA_ACT_COOKIE]) {
> + if (nla_len(tb[TCA_ACT_COOKIE]) > TC_COOKIE_MAX_SIZE) {
> + err = -EINVAL;
> + goto err_mod;
> + }
> +
> + a->act_ck = kzalloc(sizeof(*a->act_ck), GFP_KERNEL);
> + if (unlikely(!a->act_ck)) {
> + err = -ENOMEM;
> + goto err_mod;
> + }
> +
I am afraid you can't just goto err_mod for error case here, b/c ->init()
is already called before this, you probably either have to call ->destroy()
for error path, or move this before ->init().
^ permalink raw reply
* Re: [net PATCH v4 5/6] virtio: add pci_down/pci_up configuration
From: John Fastabend @ 2017-01-17 18:45 UTC (permalink / raw)
To: Jason Wang, mst; +Cc: john.r.fastabend, netdev, alexei.starovoitov, daniel
In-Reply-To: <587C464E.6020407@gmail.com>
On 17-01-15 08:04 PM, John Fastabend wrote:
> On 17-01-15 07:57 PM, Jason Wang wrote:
>>
>>
>> On 2017年01月16日 08:01, John Fastabend wrote:
>>> In virtio_net we need to do a full reset of the device to support
>>> queue reconfiguration and also we can trigger this via ethtool
>>> commands. So instead of open coding this in net driver push this
>>> into generic code in virtio. This also avoid exporting a handful
>>> of internal virtio routines.
>>
>> Looks like this is not a pci specific stuffs. And there's some driver left (e.g
>> scsi and block).
>>
>> In fact, I'm not sure touching other drivers is really needed. Maybe we can just:
>> - move virtio_device_freeze(), virtio_device_restore() and .freeze/.restore in
>> virtio_driver out of CONFIG_PM_SLEEP
>> - move virtnet_freeze() and virtnet_restore() out of CONFIG_PM_SLEEP
>> - introduce virtio_net_reset() and call
>> virtio_device_freeze()/virtio_device_restore() there
>>
>
> Michael, what do you think? I was following your suggestion on writing a
> generic virtio reset here. I don't have a preference either way.
Actually if I roll it into virtio_net per above suggestion I can probably get
the locking correct. As DaveM points out its complete junk at the moment around
the rtnl_try_lock() logic. Ugh.
I'll spin a v5...
>
>> Another possible issue for sleep/hibernation is xdp_prog were not restored, if
>> this is not XDP intended, we'd better fix this.
>
> Yep this bug exists even without this series. I'll add it to my list of things
> to fix. Along with small packet max_mtu.
>
>>
>> Thanks
>>
>> [...]
>
^ permalink raw reply
* Re: [net, 6/6] net: korina: version bump
From: Felix Fietkau @ 2017-01-17 18:55 UTC (permalink / raw)
To: Roman Yeryomin, netdev
In-Reply-To: <1484674393-22515-1-git-send-email-roman@advem.lv>
On 2017-01-17 18:33, Roman Yeryomin wrote:
> Signed-off-by: Roman Yeryomin <roman@advem.lv>
> ---
> drivers/net/ethernet/korina.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
> index 83c994f..c8fed01 100644
> --- a/drivers/net/ethernet/korina.c
> +++ b/drivers/net/ethernet/korina.c
> @@ -66,8 +66,8 @@
> #include <asm/mach-rc32434/dma_v.h>
>
> #define DRV_NAME "korina"
> -#define DRV_VERSION "0.10"
> -#define DRV_RELDATE "04Mar2008"
> +#define DRV_VERSION "0.20"
> +#define DRV_RELDATE "15Jan2017"
I think it would make more sense to remove this version instead of
bumping it. Individual driver versions are rather pointless, the kernel
version is more meaningful anyway.
- Felix
^ permalink raw reply
* Re: [PATCH net-next] tcp: accept RST for rcv_nxt - 1 after receiving a FIN
From: Eric Dumazet @ 2017-01-17 19:02 UTC (permalink / raw)
To: Jason Baron; +Cc: davem, netdev, Eric Dumazet
In-Reply-To: <1484678239-19199-1-git-send-email-jbaron@akamai.com>
On Tue, 2017-01-17 at 13:37 -0500, Jason Baron wrote:
> From: Jason Baron <jbaron@akamai.com>
>
> Using a Mac OSX box as a client connecting to a Linux server, we have found
> that when certain applications (such as 'ab'), are abruptly terminated
> (via ^C), a FIN is sent followed by a RST packet on tcp connections. The
> FIN is accepted by the Linux stack but the RST is sent with the same
> sequence number as the FIN, and Linux responds with a challenge ACK per
> RFC 5961. The OSX client then sometimes (they are rate-limited) does not
> reply with any RST as would be expected on a closed socket.
>
> This results in sockets accumulating on the Linux server left mostly in
> the CLOSE_WAIT state, although LAST_ACK and CLOSING are also possible.
> This sequence of events can tie up a lot of resources on the Linux server
> since there may be a lot of data in write buffers at the time of the RST.
> Accepting a RST equal to rcv_nxt - 1, after we have already successfully
> processed a FIN, has made a significant difference for us in practice, by
> freeing up unneeded resources in a more expedient fashion.
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks Jason !
^ permalink raw reply
* Re: [net PATCH v4 2/6] virtio_net: wrap rtnl_lock in test for calling with lock already held
From: John Fastabend @ 2017-01-17 19:03 UTC (permalink / raw)
To: David Miller
Cc: jasowang, mst, john.r.fastabend, netdev, alexei.starovoitov,
daniel
In-Reply-To: <20170117.115733.428297564258724055.davem@davemloft.net>
On 17-01-17 08:57 AM, David Miller wrote:
> From: John Fastabend <john.fastabend@gmail.com>
> Date: Sun, 15 Jan 2017 15:59:47 -0800
>
>> @@ -2358,7 +2371,10 @@ static void remove_vq_common(struct virtnet_info *vi)
>> /* Free unused buffers in both send and recv, if any. */
>> free_unused_bufs(vi);
>>
>> - free_receive_bufs(vi);
>> + if (rtnl_is_locked())
>> + _free_receive_bufs(vi);
>> + else
>> + free_receive_bufs(vi);
>>
>> free_receive_page_frags(vi);
>>
>
> This doesn't work. rtnl_is_locked() doesn't tell if _you_ own the mutex, it
> just says that someone does.
>
> So if we now execute this code without taking the RTNL lock just because some
> other thread of control holds it, we introduce a race.
>
yeah this bit is junk. dang. Trying to get this locking right without duplicate
code or pushing around lock_me variables is getting tricky.
.John
^ permalink raw reply
* Re: [PATCH net-next] tcp: accept RST for rcv_nxt - 1 after receiving a FIN
From: Rick Jones @ 2017-01-17 19:04 UTC (permalink / raw)
To: Jason Baron, davem; +Cc: netdev, Eric Dumazet
In-Reply-To: <1484678239-19199-1-git-send-email-jbaron@akamai.com>
On 01/17/2017 10:37 AM, Jason Baron wrote:
> From: Jason Baron <jbaron@akamai.com>
>
> Using a Mac OSX box as a client connecting to a Linux server, we have found
> that when certain applications (such as 'ab'), are abruptly terminated
> (via ^C), a FIN is sent followed by a RST packet on tcp connections. The
> FIN is accepted by the Linux stack but the RST is sent with the same
> sequence number as the FIN, and Linux responds with a challenge ACK per
> RFC 5961. The OSX client then sometimes (they are rate-limited) does not
> reply with any RST as would be expected on a closed socket.
>
> This results in sockets accumulating on the Linux server left mostly in
> the CLOSE_WAIT state, although LAST_ACK and CLOSING are also possible.
> This sequence of events can tie up a lot of resources on the Linux server
> since there may be a lot of data in write buffers at the time of the RST.
> Accepting a RST equal to rcv_nxt - 1, after we have already successfully
> processed a FIN, has made a significant difference for us in practice, by
> freeing up unneeded resources in a more expedient fashion.
Drifting a bit, and it doesn't change the value of dealing with it, but
out of curiosity, when you say mostly in CLOSE_WAIT, why aren't the
server-side applications reacting to the read return of zero triggered
by the arrival of the FIN?
happy benchmarking,
rick jones
^ permalink raw reply
* Re: Kernel 4.6.7-rt14 kernel workqueue lockup - rtnl deadlock plus syscall endless loop
From: David Miller @ 2017-01-17 19:05 UTC (permalink / raw)
To: EladN; +Cc: netdev
In-Reply-To: <AM5PR0701MB1873F82D61AF17A09F2B0E26A07C0@AM5PR0701MB1873.eurprd07.prod.outlook.com>
From: Elad Nachman <EladN@gilat.com>
Date: Tue, 17 Jan 2017 18:15:19 +0000
> Any thought about limiting the amount of busy polling? Say if more
> than X polls are done within a jiffy, then at least for preemptable
> kernels you can sleep for a jiffy inside the syscall to yield the
> CPU for a while?
We cannot yield there, because we must return immediately from this
context in order to drop the sysctl locks and references.
^ permalink raw reply
* Re: [PATCH] virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on xmit
From: David Miller @ 2017-01-17 19:08 UTC (permalink / raw)
To: mst; +Cc: rolf.neugebauer, netdev, jasowang, rppt
In-Reply-To: <20170117202515-mutt-send-email-mst@kernel.org>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 17 Jan 2017 20:27:33 +0200
> On Tue, Jan 17, 2017 at 06:13:51PM +0000, Rolf Neugebauer wrote:
>> This patch part reverts fd2a0437dc33 and e858fae2b0b8 which introduced a
>> subtle change in how the virtio_net flags are derived from the SKBs
>> ip_summed field.
>>
>> With the above commits, the flags are set to VIRTIO_NET_HDR_F_DATA_VALID
>> when ip_summed == CHECKSUM_UNNECESSARY, thus treating it differently to
>> ip_summed == CHECKSUM_NONE, which should be the same.
>>
>> Further, the virtio spec 1.0 / CS04 explicitly says that
>> VIRTIO_NET_HDR_F_DATA_VALID must not be set by the driver.
>>
>> Signed-off-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
>
> Fixes: fd2a0437dc33 ("virtio_net: introduce virtio_net_hdr_{from,to}_skb")
> Fixes: e858fae2b0b8 (" virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Should be backported into stable as well.
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* Re: [net, 3/6] net: korina: increase tx/rx ring sizes
From: David Miller @ 2017-01-17 19:08 UTC (permalink / raw)
To: leroi.lists; +Cc: netdev
In-Reply-To: <CACiydbLuVf_jsWor6-OJmwQhxfwd=dzBDnDQnJrp3owf8qCkQA@mail.gmail.com>
From: Roman Yeryomin <leroi.lists@gmail.com>
Date: Tue, 17 Jan 2017 20:27:57 +0200
> On 17 January 2017 at 19:58, David Miller <davem@davemloft.net> wrote:
>> From: Roman Yeryomin <leroi.lists@gmail.com>
>> Date: Tue, 17 Jan 2017 19:32:36 +0200
>>
>>> Having larger ring sizes almost eliminates rx fifo overflow, thus improving performance.
>>> This patch reduces rx overflow occurence by approximately 1000 times (from ~25k down to ~25 times per 3M frames)
>>
>> Those numbers don't mean much without full context.
>>
>> What kind of system, what kind of traffic, and over what kind of link?
>
> MIPS rb532 board, TCP iperf3 test over 100M link, NATed speed ~55Mbps.
> I can do more tests and provide more precise numbers, if needed.
Provide this information in the commit message on your respin of this patch
series after you've addressed all of the other feedback, don't tell me :-)
^ permalink raw reply
* Re: [net, 2/6] net: korina: define NAPI_WEIGHT
From: David Miller @ 2017-01-17 19:08 UTC (permalink / raw)
To: leroi.lists; +Cc: netdev
In-Reply-To: <CACiydbLyswWZ2ZX04m22Ga_FBbp-52kxOxBbNcvS+R1d1Kdpcw@mail.gmail.com>
From: Roman Yeryomin <leroi.lists@gmail.com>
Date: Tue, 17 Jan 2017 20:30:44 +0200
> On 17 January 2017 at 19:40, David Miller <davem@davemloft.net> wrote:
>> From: Roman Yeryomin <leroi.lists@gmail.com>
>> Date: Tue, 17 Jan 2017 19:32:22 +0200
>>
>>> @@ -77,6 +77,8 @@
>>>
>>> #define MII_CLOCK 1250000 /* no more than 2.5MHz */
>>>
>>> +#define NAPI_WEIGHT 64
>>> +
>>> /* the following must be powers of two */
>>> #define KORINA_NUM_RDS 64 /* number of receive descriptors */
>>> #define KORINA_NUM_TDS 64 /* number of transmit descriptors */
>>> @@ -1080,7 +1082,7 @@ static int korina_probe(struct platform_device *pdev)
>>> dev->netdev_ops = &korina_netdev_ops;
>>> dev->ethtool_ops = &netdev_ethtool_ops;
>>> dev->watchdog_timeo = TX_TIMEOUT;
>>> - netif_napi_add(dev, &lp->napi, korina_poll, 64);
>>> + netif_napi_add(dev, &lp->napi, korina_poll, NAPI_WEIGHT);
>>
>> Please use NAPI_POLL_WEIGHT from linux/netdevice.h
>
> OK, should I resend the whole set or just this one and dependant?
You always must do this for any change whatsoever, a full respin of a
series is always required on any modification whatsoever to any patch.
^ 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;
as well as URLs for NNTP newsgroup(s).