* Re: [RFT Patch net 1/2] ipv6: invalidate the socket cached route on pmtu events if possible
From: Eric Dumazet @ 2016-03-25 1:51 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Wei Wang, Steffen Klassert,
Martin KaFai Lau, Hannes Frederic Sowa, Julian Anastasov
In-Reply-To: <CAM_iQpVDN_LPsa2XgVWoqMjrOYpg9JNjPJVwEvTuxPgYyWRWmg@mail.gmail.com>
On Thu, 2016-03-24 at 17:15 -0700, Cong Wang wrote:
> My understanding is that bh_lock_sock() prevents concurrent
> access to sock struct. Since this is in softirq context, multiple
> CPU's could call this function concurrently, the whole pmtu
> update needs to be done atomically.
>
> UDP, on the other hand, doesn't do this logic, it just looks up
> for dst and save it in sk_dst_cache.
Two ICMP messages processed on two different cpus can still get two
different sockets pointing to the same dst.
I do not see how dst pmtu update could be protected by a lock on each
socket. It would require a dst lock , or some simple memory writes that
do not require special synchro.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Vijay Pandurangan @ 2016-03-25 1:44 UTC (permalink / raw)
To: Ben Greear; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <56F490B2.3090603@candelatech.com>
Oops, I think my last email didn't go through due to an inadvertent
html attachment from my phone mail client.
Can you send us a copy of a packet you're sending and/or confirm that
the IP and UDP4 checksums are set correctly in the packet?
If those are set right, I think we need to read through the networking
code again to see why this is broken...
On Thu, Mar 24, 2016 at 9:13 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 03/24/2016 06:11 PM, Ben Greear wrote:
>>
>> On 03/24/2016 05:06 PM, Ben Greear wrote:
>>>
>>> On 03/24/2016 04:56 PM, Cong Wang wrote:
>>>>
>>>> On Thu, Mar 24, 2016 at 3:01 PM, Ben Greear <greearb@candelatech.com>
>>>> wrote:
>>>>>
>>>>> I have an application that creates two pairs of veth devices.
>>>>>
>>>>> a <-> b c <-> d
>>>>>
>>>>> b and c have a raw packet socket opened on them and I 'bridge' frames
>>>>> between b and c to provide network emulation (ie, configurable delay).
>>>>>
>>>>
>>>> IIUC, you create two raw sockets in order to bridge these two veth
>>>> pairs?
>>>> That is, to receive packets on one socket and deliver packets on the
>>>> other?
>>>
>>>
>>> Yes.
>>>
>>>>> I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP
>>>>> connection
>>>>> (using policy based routing to ensure frames are sent on the
>>>>> appropriate
>>>>> interfaces).
>>>>>
>>>>> This is user-space only app, and kernel in this case is completely
>>>>> unmodified.
>>>>>
>>>>> The commit below breaks this feature: UDP frames are sniffed on both a
>>>>> and
>>>>> d ports
>>>>> (in both directions), but the UDP socket does not receive frames.
>>>>>
>>>>> Using normal ethernet ports, this network emulation feature works fine,
>>>>> so
>>>>> it is
>>>>> specific to VETH.
>>>>>
>>>>> A similar test with just sending UDP between a single veth pair: e <->
>>>>> f
>>>>> works fine. Maybe it has something to do with raw packets?
>>>>>
>>>>
>>>> Yeah, I have the same feeling. Could you trace kfree_skb() to see
>>>> where these packets are dropped? At UDP layer?
>>>
>>>
>>> Since reverting the patch fixes this, it almost certainly has to be due
>>> to some
>>> checksum checking logic. Since UDP sockets (between single veth pair)
>>> works, it would appear to be related to my packet bridge, so maybe
>>> it is specific to raw packets and/or sendmmsg api.
>>>
>>> I'll investigate it better tomorrow.
>>
>>
>> So, I found time to poke at it this evening:
>>
>> Sending between two veth pairs, no packet bridge involved.
>
>
> Errrr, to be clear: I mean sending between two ends of a single veth pair
> here.
>
>>
>> UDP: ip_summed is 3 (CHECKSUM_PARTIAL) # Works fine.
>> raw packet frames, custom ether protocol (0x1111 type): ip_summed is 0
>> (NONE) # Works fine.
>>
>> When I try to send UDP through the veth pairs & pkt bridge, I see this:
>
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
>
^ permalink raw reply
* Re: [RFC PATCH] tcp: Add SOF_TIMESTAMPING_TX_EOR and allow MSG_EOR in tcp_sendmsg
From: Willem de Bruijn @ 2016-03-25 1:39 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Network Development, Kernel Team, Eric Dumazet, Neal Cardwell,
Willem de Bruijn, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <1458859592-751521-1-git-send-email-kafai@fb.com>
> This patch allows the user process to use MSG_EOR during
> tcp_sendmsg to tell the kernel that it is the last byte
> of an application response message.
>
> The user process can use the new SOF_TIMESTAMPING_TX_EOR to
> ask the kernel to only track timestamp of the MSG_EOR byte.
Selective timestamp requests is a useful addition. Soheil (cc-ed) also
happens to be looking at this. That approach uses cmsg to selectively
tag send calls, avoiding the need to define a new SOF_ flag.
> The current SOF_TIMESTAMPING_TX_ACK is tracking the last
> byte appended to a skb during the tcp_sendmsg. It may track
> multiple bytes if the response spans across multiple skbs.
It only tracks the last byte of the buffer passed in sendmsg. If a
sendmsg results in multiple skbuffs, only the last skb is tracked. It
is, however, possible that that skbuff is appended to in a follow-on
sendmsg call. If multiple calls enable recording on an skbuff, only
the last record request on an skb is kept.
> it is enough to measure the response latency for application
> protocol with a single request/response at a time (like HTTP 1.1
> without pipeline), it does not work well for application protocol
> with >1 pipeline responses (like HTTP2).
>
> Each skb can only track one tskey (which is the seq number of
> the last byte of the message). To allow tracking the
> last byte of multiple response messages, this patch takes an approach
> by not appending to the last skb during tcp_sendmsg if the last skb's
> tskey will be overwritten. A similar case also happens during retransmit.
>
> This approach avoids introducing another list to track the tskey. The
> downside is that it will have less gso benefit and/or more outgoing
> packets. Practically, due to the amount of measurement data generated,
> sampling is usually used in production. (i.e. not every connection is
> tracked).
Agreed. This is the simplest approach to avoiding timestamp request
overwrites. A list-based approach quickly becomes complex as skbuffs
can be split and merged at various points.
Since this use is rare, I would suggest making the code even simpler
by just jumping to new_segment on a call with this MSG option (or
cmsg) set, avoiding tcp_tx_ts_noappend_skb() on each new segment.
> One of our use case is at the webserver. The webserver tracks
> the HTTP2 response latency by measuring when the webserver
> sends the first byte to the socket till the TCP ACK of the last byte
> is received. In the cases where we don't have client side
> measurement, measuring from the server side is the only option.
> In the cases we have the client side measurement, the server side
> data can also be used to justify/cross-check-with the client
> side data (e.g. is there slowness at the layer above the client's
> TCP stack).
>
> The TCP PRR paper [1] also measures a similar metrics:
> "The TCP latency of a HTTP response when the server sends the first
> byte until it receives the acknowledgment (ACK) for the last byte."
>
> [1] Proportional Rate Reduction for TCP:
> http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/37486.pdf
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> ---
> include/uapi/linux/net_tstamp.h | 3 ++-
> net/ipv4/tcp.c | 23 ++++++++++++++++++-----
> net/ipv4/tcp_output.c | 9 +++++++--
> 3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
> index 6d1abea..5376569 100644
> --- a/include/uapi/linux/net_tstamp.h
> +++ b/include/uapi/linux/net_tstamp.h
> @@ -25,8 +25,9 @@ enum {
> SOF_TIMESTAMPING_TX_ACK = (1<<9),
> SOF_TIMESTAMPING_OPT_CMSG = (1<<10),
> SOF_TIMESTAMPING_OPT_TSONLY = (1<<11),
> + SOF_TIMESTAMPING_TX_EOR = (1<<12),
>
> - SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_TSONLY,
> + SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_EOR,
> SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
> SOF_TIMESTAMPING_LAST
> };
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 08b8b96..7de96eb 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -428,11 +428,16 @@ void tcp_init_sock(struct sock *sk)
> }
> EXPORT_SYMBOL(tcp_init_sock);
>
> -static void tcp_tx_timestamp(struct sock *sk, struct sk_buff *skb)
> +static void tcp_tx_timestamp(struct sock *sk, struct sk_buff *skb, int flags)
> {
> if (sk->sk_tsflags) {
> - struct skb_shared_info *shinfo = skb_shinfo(skb);
> + struct skb_shared_info *shinfo;
>
> + if ((sk->sk_tsflags & SOF_TIMESTAMPING_TX_EOR) &&
> + !(flags & MSG_EOR))
> + return;
> +
> + shinfo = skb_shinfo(skb);
> sock_tx_timestamp(sk, &shinfo->tx_flags);
> if (shinfo->tx_flags & SKBTX_ANY_TSTAMP)
> shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
> @@ -957,7 +962,7 @@ new_segment:
> offset += copy;
> size -= copy;
> if (!size) {
> - tcp_tx_timestamp(sk, skb);
> + tcp_tx_timestamp(sk, skb, flags);
> goto out;
> }
>
> @@ -1073,6 +1078,14 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
> return err;
> }
>
> +static bool tcp_tx_ts_noappend_skb(const struct sock *sk,
> + const struct sk_buff *last_skb, int flags)
> +{
> + return unlikely((sk->sk_tsflags & SOF_TIMESTAMPING_TX_EOR) &&
> + (flags & MSG_EOR) &&
flags seems more likely to be cached than sk->sk_tsflags at this
point, in which case swap those tests.
> + (skb_shinfo(last_skb)->tx_flags & SKBTX_ANY_TSTAMP));
> +}
> +
> int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
for a non-rfc patch, also change do_tcp_sendpages
> {
> struct tcp_sock *tp = tcp_sk(sk);
> @@ -1144,7 +1157,7 @@ int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> copy = max - skb->len;
> }
>
> - if (copy <= 0) {
> + if (copy <= 0 || tcp_tx_ts_noappend_skb(sk, skb, flags)) {
> new_segment:
This adds a test to every segment for a niche feature. See my point of
just jumping here on first entering the loop.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 1:13 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Evan Jones, Vijay P, Cong Wang
In-Reply-To: <56F49036.8050902@candelatech.com>
On 03/24/2016 06:11 PM, Ben Greear wrote:
> On 03/24/2016 05:06 PM, Ben Greear wrote:
>> On 03/24/2016 04:56 PM, Cong Wang wrote:
>>> On Thu, Mar 24, 2016 at 3:01 PM, Ben Greear <greearb@candelatech.com> wrote:
>>>> I have an application that creates two pairs of veth devices.
>>>>
>>>> a <-> b c <-> d
>>>>
>>>> b and c have a raw packet socket opened on them and I 'bridge' frames
>>>> between b and c to provide network emulation (ie, configurable delay).
>>>>
>>>
>>> IIUC, you create two raw sockets in order to bridge these two veth pairs?
>>> That is, to receive packets on one socket and deliver packets on the other?
>>
>> Yes.
>>
>>>> I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP connection
>>>> (using policy based routing to ensure frames are sent on the appropriate
>>>> interfaces).
>>>>
>>>> This is user-space only app, and kernel in this case is completely
>>>> unmodified.
>>>>
>>>> The commit below breaks this feature: UDP frames are sniffed on both a and
>>>> d ports
>>>> (in both directions), but the UDP socket does not receive frames.
>>>>
>>>> Using normal ethernet ports, this network emulation feature works fine, so
>>>> it is
>>>> specific to VETH.
>>>>
>>>> A similar test with just sending UDP between a single veth pair: e <-> f
>>>> works fine. Maybe it has something to do with raw packets?
>>>>
>>>
>>> Yeah, I have the same feeling. Could you trace kfree_skb() to see
>>> where these packets are dropped? At UDP layer?
>>
>> Since reverting the patch fixes this, it almost certainly has to be due to some
>> checksum checking logic. Since UDP sockets (between single veth pair)
>> works, it would appear to be related to my packet bridge, so maybe
>> it is specific to raw packets and/or sendmmsg api.
>>
>> I'll investigate it better tomorrow.
>
> So, I found time to poke at it this evening:
>
> Sending between two veth pairs, no packet bridge involved.
Errrr, to be clear: I mean sending between two ends of a single veth pair here.
>
> UDP: ip_summed is 3 (CHECKSUM_PARTIAL) # Works fine.
> raw packet frames, custom ether protocol (0x1111 type): ip_summed is 0 (NONE) # Works fine.
>
> When I try to send UDP through the veth pairs & pkt bridge, I see this:
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 1:11 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Evan Jones, Vijay P, Cong Wang
In-Reply-To: <56F4810A.9060904@candelatech.com>
On 03/24/2016 05:06 PM, Ben Greear wrote:
> On 03/24/2016 04:56 PM, Cong Wang wrote:
>> On Thu, Mar 24, 2016 at 3:01 PM, Ben Greear <greearb@candelatech.com> wrote:
>>> I have an application that creates two pairs of veth devices.
>>>
>>> a <-> b c <-> d
>>>
>>> b and c have a raw packet socket opened on them and I 'bridge' frames
>>> between b and c to provide network emulation (ie, configurable delay).
>>>
>>
>> IIUC, you create two raw sockets in order to bridge these two veth pairs?
>> That is, to receive packets on one socket and deliver packets on the other?
>
> Yes.
>
>>> I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP connection
>>> (using policy based routing to ensure frames are sent on the appropriate
>>> interfaces).
>>>
>>> This is user-space only app, and kernel in this case is completely
>>> unmodified.
>>>
>>> The commit below breaks this feature: UDP frames are sniffed on both a and
>>> d ports
>>> (in both directions), but the UDP socket does not receive frames.
>>>
>>> Using normal ethernet ports, this network emulation feature works fine, so
>>> it is
>>> specific to VETH.
>>>
>>> A similar test with just sending UDP between a single veth pair: e <-> f
>>> works fine. Maybe it has something to do with raw packets?
>>>
>>
>> Yeah, I have the same feeling. Could you trace kfree_skb() to see
>> where these packets are dropped? At UDP layer?
>
> Since reverting the patch fixes this, it almost certainly has to be due to some
> checksum checking logic. Since UDP sockets (between single veth pair)
> works, it would appear to be related to my packet bridge, so maybe
> it is specific to raw packets and/or sendmmsg api.
>
> I'll investigate it better tomorrow.
So, I found time to poke at it this evening:
Sending between two veth pairs, no packet bridge involved.
UDP: ip_summed is 3 (CHECKSUM_PARTIAL) # Works fine.
raw packet frames, custom ether protocol (0x1111 type): ip_summed is 0 (NONE) # Works fine.
When I try to send UDP through the veth pairs & pkt bridge, I see this:
(pkt-bridge connects to the 'b' side of the veth pairs)
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR0 rcv: rddVR0b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR1b rcv: rddVR1 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR1 rcv: rddVR1b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR0b rcv: rddVR0 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR0 rcv: rddVR0b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:34 ben-ota-1 kernel: dev: rddVR1b rcv: rddVR1 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR1 rcv: rddVR1b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0b rcv: rddVR0 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0 rcv: rddVR0b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR1b rcv: rddVR1 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR1 rcv: rddVR1b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0b rcv: rddVR0 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0 rcv: rddVR0b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR1b rcv: rddVR1 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR1 rcv: rddVR1b ip_summed: 3 rcv-features: 0x184074011e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0b rcv: rddVR0 ip_summed: 0 rcv-features: 0x184075b59e9
Mar 24 17:59:35 ben-ota-1 kernel: dev: rddVR0 rcv: rddVR0b ip_summed: 3 rcv-features: 0x184074011e9
I am guessing the issue is that when my pkt bridge sends a raw frame that is actually a UDP packet,
the fact that it has ip_summed == 0 in the kernel causes the frame to be dropped.
I modified veth.c like this for this test:
static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
struct net_device *rcv;
int length = skb->len;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
if (unlikely(!rcv)) {
kfree_skb(skb);
goto drop;
}
pr_err("dev: %s rcv: %s ip_summed: %d rcv-features: 0x%llx\n",
dev->name, rcv->name, skb->ip_summed, (unsigned long long)rcv->features);
#if 0
/* don't change ip_summed == CHECKSUM_PARTIAL, as that
* will cause bad checksum on forwarded packets
*/
if (skb->ip_summed == CHECKSUM_NONE &&
rcv->features & NETIF_F_RXCSUM)
skb->ip_summed = CHECKSUM_UNNECESSARY;
#endif
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Edward Cree @ 2016-03-25 0:37 UTC (permalink / raw)
To: Alexander Duyck
Cc: Edward Cree, Or Gerlitz, Alexander Duyck, Netdev, David Miller,
Tom Herbert
In-Reply-To: <CAKgT0UcoVV-_LrLPRoK4PLim5Nn-Z7gOV9LZF_RMXDOU8aCciA@mail.gmail.com>
On Thu, 24 Mar 2016, Alexander Duyck wrote:
> On Thu, Mar 24, 2016 at 4:00 PM, Edward Cree <ecree@solarflare.com> wrote:
>> So: in the initial transmit path we build a coherent superframe; when we get
>> to the device, we say either "oh, device doesn't support offloads at all,
>> call GSO", or "oh, device supports this particular GSO type for TSO, so give
>> it the superframe SKB to do what it wants with", or "oh, device supports
>> GSO_PARTIAL, let's call skb_mac_gso_partial [i.e. chain of new callbacks I
>> was describing in previous email] and then give the non-coherent SKB to the
>> device".
>
> What you are proposing is essentially forking GSO.
In a way, I suppose it is.
> I really don't
> like that. I prefer the approach we have now where essentially GSO
> partial is something checked for at the very end and we have a few
> checks when putting together the headers so we do the right thing.
I guess both can exist, and I'll name mine something other than "GSO
partial"...
> This is sounding very complicated. I think we would be better off if
> you took the time to try and implement some of this yourself so you
> could see how feasible it is.
Sure, I was already intending to do that before I saw your patch series
and thought it might be able to do what I had in mind. As it now seems
like we're envisaging different things, I'll go back to implementing
mine and wish you luck with yours.
-Ed
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: David Miller @ 2016-03-25 0:25 UTC (permalink / raw)
To: eric.dumazet
Cc: ylavic.dev, netdev, tom, w, tolga.ceylan, cgallek, josh, aconole,
daniel
In-Reply-To: <1458859788.6473.2.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Mar 2016 15:49:48 -0700
> That is why EBPF has LLVM backend.
>
> Basically you can write your "BPF" program in C, and let llvm convert it
> into EBPF.
>
> Sure, you still can write BPF manually, as you could write HTTPS server
> in assembly.
+1
^ permalink raw reply
* Re: [net-next RFC 0/4] SO_BINDTOSUBNET
From: Tom Herbert @ 2016-03-25 0:25 UTC (permalink / raw)
To: Gilberto Bertin; +Cc: Linux Kernel Network Developers
In-Reply-To: <1458134349-2454-1-git-send-email-gilberto.bertin@gmail.com>
On Wed, Mar 16, 2016 at 6:19 AM, Gilberto Bertin
<gilberto.bertin@gmail.com> wrote:
> This is my second attempt to submit an RFC for this patch.
>
> Some arguments for and against it since the first submission:
> * SO_BINDTOSUBNET is an arbitrary option and can be seens as nother use
> * case of the SO_REUSEPORT BPF patch
> * but at the same time using BPF requires more work/code on the server
> and since the bind to subnet use case could potentially become a
> common one maybe there is some value in having it as an option instead
> of having to code (either manually or with clang) an eBPF program that
> would do the same
Gilberto, I'm not sure I understand this argument. Have you
implemented the BPF bind solution?
Thanks,
Tom
> * it may probably possible to archive the same results using VRF. This
> would require to create a VRF device, configure the device routing
> table and make each bind each process to a different VRF device (but
> I'm not sure how this would work/interfere with an existing iptables
> setup for example)
>
> -----------------------------------------------------------------------------
>
> This series introduces support for the SO_BINDTOSUBNET socket option, which
> allows a listener socket to bind to a subnet instead of * or a single address.
>
> Motivation:
> consider a set of servers, each one with thousands and thousands of IP
> addresses. Since assigning /32 or /128 IP individual addresses would be
> inefficient, one solution can be assigning subnets using local routes
> (with 'ip route add local').
>
> This allows a listener to listen and terminate connections going to any
> of the IP addresses of these subnets without explicitly configuring all
> the IP addresses of the subnet range.
> This is very efficient.
>
> Unfortunately there may be the need to use different subnets for
> different purposes.
> One can imagine port 80 being served by one HTTP server for some IP
> subnet, while another server used for another subnet.
> Right now Linux does not allow this.
> It is either possible to bind to *, indicating ALL traffic going to
> given port, or to individual IP addresses.
> The first only allows to accept connections from all the subnets.
> The latter does not scale well with lots of IP addresses.
>
> Using bindtosubnet would solve this problem: just by adding a local
> route rule and setting the SO_BINDTOSUBNET option for a socket it would
> be possible to easily partition traffic by subnets.
>
> API:
> the subnet is specified (as argument of the setsockopt syscall) by the
> address of the network, and the prefix length of the netmask.
>
> IPv4:
> struct ipv4_subnet {
> __be32 net;
> u_char plen;
> };
>
> and IPv6:
> struct ipv6_subnet {
> struct in6_addr net;
> u_char plen;
> };
>
> Bind conflicts:
> two sockets with the bindtosubnet option enabled generate a bind
> conflict if their network addresses masked with the shortest of their
> prefix are equal.
> The bindtosubnet option can be combined with soreuseport so that two
> listener can bind on the same subnet.
>
> Any questions/feedback appreciated.
>
> Thanks,
> Gilberto
>
> Gilberto Bertin (4):
> bindtosubnet: infrastructure
> bindtosubnet: TCP/IPv4 implementation
> bindtosubnet: TCP/IPv6 implementation
> bindtosubnet: UPD implementation
>
> include/net/sock.h | 20 +++++++
> include/uapi/asm-generic/socket.h | 1 +
> net/core/sock.c | 111 ++++++++++++++++++++++++++++++++++++++
> net/ipv4/inet_connection_sock.c | 20 ++++++-
> net/ipv4/inet_hashtables.c | 9 ++++
> net/ipv4/udp.c | 36 +++++++++++++
> net/ipv6/inet6_connection_sock.c | 17 +++++-
> net/ipv6/inet6_hashtables.c | 6 +++
> 8 files changed, 218 insertions(+), 2 deletions(-)
>
> --
> 2.7.2
>
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: David Miller @ 2016-03-25 0:24 UTC (permalink / raw)
To: ylavic.dev
Cc: netdev, tom, eric.dumazet, w, tolga.ceylan, cgallek, josh,
aconole, daniel
In-Reply-To: <CAKQ1sVP_bbGYnVW=9WDnuzJ7qcC1xrM3qmeYFk-c3fi696F5-g@mail.gmail.com>
From: Yann Ylavic <ylavic.dev@gmail.com>
Date: Thu, 24 Mar 2016 23:40:30 +0100
> On Thu, Mar 24, 2016 at 6:55 PM, Daniel Borkmann wrote:
>> On 03/24/2016 06:26 PM, Tom Herbert wrote:
>>>
>>> On Thu, Mar 24, 2016 at 10:01 AM, Eric Dumazet wrote:
>>>>
>>>> Really, when BPF can be the solution, we wont allow adding new stuff in
>>>> the kernel in the old way.
>>>
>>> I completely agree with this, but I wonder if we now need a repository
>>> of useful BPF modules. So in the case of implementing functionality
>>> like in SO_REUSEPORT_LISTEN_OFF that might just become a common BPF
>>> program we could direct people to use.
>>
>> Good point. There's tools/testing/selftests/net/ containing already
>> reuseport
>> BPF example, maybe it could be extended.
>
> FWIW, I find:
>
> const struct bpf_insn prog[] = {
> /* BPF_MOV64_REG(BPF_REG_6, BPF_REG_1) */
> { BPF_ALU64 | BPF_MOV | BPF_X, BPF_REG_6, BPF_REG_1, 0, 0 },
> /* BPF_LD_ABS(BPF_W, 0) R0 = (uint32_t)skb[0] */
> { BPF_LD | BPF_ABS | BPF_W, 0, 0, 0, 0 },
> /* BPF_ALU64_IMM(BPF_MOD, BPF_REG_0, mod) */
> { BPF_ALU64 | BPF_MOD | BPF_K, BPF_REG_0, 0, 0, mod },
> /* BPF_EXIT_INSN() */
> { BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
> };
> (and all the way to make it run)
>
> something quite unintuitive from a web server developper perspective,
> simply to make SO_REUSEPORT work with forked TCP listeners (probably
> as it should out of the box)...
If we encapsulate this into libraries and helper wrappers, there is
no reason web server developers should be looking at these details
anyways.
Please don't make a mountain out of a mole-hill.
We build things on top of good infrastructure, rather than build
duplicate ways to do the same exact thing.
BPF is good infrastructure, therefore that is what things will be
built on top of.
Thanks.
^ permalink raw reply
* Re: [RFT Patch net 1/2] ipv6: invalidate the socket cached route on pmtu events if possible
From: Cong Wang @ 2016-03-25 0:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Wei Wang, Steffen Klassert,
Martin KaFai Lau, Hannes Frederic Sowa, Julian Anastasov
In-Reply-To: <1458851740.12033.24.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Mar 24, 2016 at 1:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-03-24 at 12:25 -0700, Cong Wang wrote:
>
>> void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu)
>> {
>> - ip6_update_pmtu(skb, sock_net(sk), mtu,
>> - sk->sk_bound_dev_if, sk->sk_mark);
>> + const struct ipv6hdr *iph = (struct ipv6hdr *)skb->data;
>> + struct net *net = sock_net(sk);
>> + struct dst_entry *ndst, *dst;
>> + struct flowi6 fl6;
>> + bool new = false;
>> +
>> + memset(&fl6, 0, sizeof(fl6));
>> +
>> + bh_lock_sock(sk);
>> +
>
>
> This is not clear why you need to acquire socket lock.
>
> sk_dst_cache is protected by an atomic operation.
>
> udp for example calls ip6_dst_store() without socket being locked.
My understanding is that bh_lock_sock() prevents concurrent
access to sock struct. Since this is in softirq context, multiple
CPU's could call this function concurrently, the whole pmtu
update needs to be done atomically.
UDP, on the other hand, doesn't do this logic, it just looks up
for dst and save it in sk_dst_cache.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 0:06 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Evan Jones, Vijay P, Cong Wang
In-Reply-To: <CAM_iQpWv_iP1ZtsRQT0YsztMrf6rdaXN2V57PUu5oBe5kR56AQ@mail.gmail.com>
On 03/24/2016 04:56 PM, Cong Wang wrote:
> On Thu, Mar 24, 2016 at 3:01 PM, Ben Greear <greearb@candelatech.com> wrote:
>> I have an application that creates two pairs of veth devices.
>>
>> a <-> b c <-> d
>>
>> b and c have a raw packet socket opened on them and I 'bridge' frames
>> between b and c to provide network emulation (ie, configurable delay).
>>
>
> IIUC, you create two raw sockets in order to bridge these two veth pairs?
> That is, to receive packets on one socket and deliver packets on the other?
Yes.
>> I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP connection
>> (using policy based routing to ensure frames are sent on the appropriate
>> interfaces).
>>
>> This is user-space only app, and kernel in this case is completely
>> unmodified.
>>
>> The commit below breaks this feature: UDP frames are sniffed on both a and
>> d ports
>> (in both directions), but the UDP socket does not receive frames.
>>
>> Using normal ethernet ports, this network emulation feature works fine, so
>> it is
>> specific to VETH.
>>
>> A similar test with just sending UDP between a single veth pair: e <-> f
>> works fine. Maybe it has something to do with raw packets?
>>
>
> Yeah, I have the same feeling. Could you trace kfree_skb() to see
> where these packets are dropped? At UDP layer?
Since reverting the patch fixes this, it almost certainly has to be due to some
checksum checking logic. Since UDP sockets (between single veth pair)
works, it would appear to be related to my packet bridge, so maybe
it is specific to raw packets and/or sendmmsg api.
I'll investigate it better tomorrow.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Yann Ylavic @ 2016-03-25 0:01 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, Linux Kernel Network Developers, Willy Tarreau,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <CALx6S37uZah89sNgH9wuD1J+_WEhd34Z5zmrnG8Qp-AQ7Ew=Jg@mail.gmail.com>
On Fri, Mar 25, 2016 at 12:54 AM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, Mar 24, 2016 at 4:40 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>
>> From this POV, draining the (ending) listeners is already non obvious
>> but might be reasonable, (e)BPF sounds really overkill.
>>
> Just the opposite, it's a simplification. With BPF we no longer to add
> interfaces for all these special cases. This is an important point,
> because the question is going to be raised for any proposed interface
> change that could be accomplished with BPF (i.e. adding new interfaces
> in the kernel becomes the overkill).
>
> Please try to work with it. As I mentioned, the part that we may be
> missing are some real world programs that we can direct people to use,
> but aside from that I don't think we've seen any arguments that BPF is
> overkill or too hard to use for stuff like this.
OK, you certainly know better than me.
Really looking forward these real world BPF programs to enlighten me.
Thanks for your time.
Regards,
Yann.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Cong Wang @ 2016-03-24 23:56 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev, Evan Jones, Vijay P, Cong Wang
In-Reply-To: <56F463D6.7080406@candelatech.com>
On Thu, Mar 24, 2016 at 3:01 PM, Ben Greear <greearb@candelatech.com> wrote:
> I have an application that creates two pairs of veth devices.
>
> a <-> b c <-> d
>
> b and c have a raw packet socket opened on them and I 'bridge' frames
> between b and c to provide network emulation (ie, configurable delay).
>
IIUC, you create two raw sockets in order to bridge these two veth pairs?
That is, to receive packets on one socket and deliver packets on the other?
>
> I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP connection
> (using policy based routing to ensure frames are sent on the appropriate
> interfaces).
>
> This is user-space only app, and kernel in this case is completely
> unmodified.
>
> The commit below breaks this feature: UDP frames are sniffed on both a and
> d ports
> (in both directions), but the UDP socket does not receive frames.
>
> Using normal ethernet ports, this network emulation feature works fine, so
> it is
> specific to VETH.
>
> A similar test with just sending UDP between a single veth pair: e <-> f
> works fine. Maybe it has something to do with raw packets?
>
Yeah, I have the same feeling. Could you trace kfree_skb() to see
where these packets are dropped? At UDP layer?
Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Tom Herbert @ 2016-03-24 23:54 UTC (permalink / raw)
To: Yann Ylavic
Cc: Eric Dumazet, Linux Kernel Network Developers, Willy Tarreau,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <CAKQ1sVPJoaaf1gRfgYPRygfMFVqk08Q0K74wn6kLHV+vy=8w1w@mail.gmail.com>
On Thu, Mar 24, 2016 at 4:40 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
> On Thu, Mar 24, 2016 at 11:49 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Thu, 2016-03-24 at 23:40 +0100, Yann Ylavic wrote:
>>
>>> FWIW, I find:
>>>
>>> const struct bpf_insn prog[] = {
>>> /* BPF_MOV64_REG(BPF_REG_6, BPF_REG_1) */
>>> { BPF_ALU64 | BPF_MOV | BPF_X, BPF_REG_6, BPF_REG_1, 0, 0 },
>>> /* BPF_LD_ABS(BPF_W, 0) R0 = (uint32_t)skb[0] */
>>> { BPF_LD | BPF_ABS | BPF_W, 0, 0, 0, 0 },
>>> /* BPF_ALU64_IMM(BPF_MOD, BPF_REG_0, mod) */
>>> { BPF_ALU64 | BPF_MOD | BPF_K, BPF_REG_0, 0, 0, mod },
>>> /* BPF_EXIT_INSN() */
>>> { BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
>>> };
>>> (and all the way to make it run)
>>>
>>> something quite unintuitive from a web server developper perspective,
>>> simply to make SO_REUSEPORT work with forked TCP listeners (probably
>>> as it should out of the box)...
>>
>>
>> That is why EBPF has LLVM backend.
>>
>> Basically you can write your "BPF" program in C, and let llvm convert it
>> into EBPF.
>
> I'll learn how to do this to get the best performances from the
> server, but having to do so to work around what looks like a defect
> (for simple/default SMP configurations at least, no NUMA or clever
> CPU-affinity or queuing policy involved) seems odd in the first place.
>
I disagree with your assessment that there is a defect. SO_REUSEPORT
is designed to spread packets amongst _equivalent_ connections. In the
server draining case sockets are no longer equivalent, but that is a
special case.
> From this POV, draining the (ending) listeners is already non obvious
> but might be reasonable, (e)BPF sounds really overkill.
>
Just the opposite, it's a simplification. With BPF we no longer to add
interfaces for all these special cases. This is an important point,
because the question is going to be raised for any proposed interface
change that could be accomplished with BPF (i.e. adding new interfaces
in the kernel becomes the overkill).
Please try to work with it. As I mentioned, the part that we may be
missing are some real world programs that we can direct people to use,
but aside from that I don't think we've seen any arguments that BPF is
overkill or too hard to use for stuff like this.
Tom
> But there are surely plenty of good reasons for it, and I won't be
> able to dispute your technical arguments in any case ;)
>
>>
>> Sure, you still can write BPF manually, as you could write HTTPS server
>> in assembly.
>
> OK, I'll take your previous proposal :)
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Yann Ylavic @ 2016-03-24 23:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Tom Herbert, Willy Tarreau,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <1458859788.6473.2.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Mar 24, 2016 at 11:49 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-03-24 at 23:40 +0100, Yann Ylavic wrote:
>
>> FWIW, I find:
>>
>> const struct bpf_insn prog[] = {
>> /* BPF_MOV64_REG(BPF_REG_6, BPF_REG_1) */
>> { BPF_ALU64 | BPF_MOV | BPF_X, BPF_REG_6, BPF_REG_1, 0, 0 },
>> /* BPF_LD_ABS(BPF_W, 0) R0 = (uint32_t)skb[0] */
>> { BPF_LD | BPF_ABS | BPF_W, 0, 0, 0, 0 },
>> /* BPF_ALU64_IMM(BPF_MOD, BPF_REG_0, mod) */
>> { BPF_ALU64 | BPF_MOD | BPF_K, BPF_REG_0, 0, 0, mod },
>> /* BPF_EXIT_INSN() */
>> { BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
>> };
>> (and all the way to make it run)
>>
>> something quite unintuitive from a web server developper perspective,
>> simply to make SO_REUSEPORT work with forked TCP listeners (probably
>> as it should out of the box)...
>
>
> That is why EBPF has LLVM backend.
>
> Basically you can write your "BPF" program in C, and let llvm convert it
> into EBPF.
I'll learn how to do this to get the best performances from the
server, but having to do so to work around what looks like a defect
(for simple/default SMP configurations at least, no NUMA or clever
CPU-affinity or queuing policy involved) seems odd in the first place.
>From this POV, draining the (ending) listeners is already non obvious
but might be reasonable, (e)BPF sounds really overkill.
But there are surely plenty of good reasons for it, and I won't be
able to dispute your technical arguments in any case ;)
>
> Sure, you still can write BPF manually, as you could write HTTPS server
> in assembly.
OK, I'll take your previous proposal :)
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-24 23:35 UTC (permalink / raw)
To: Edward Cree
Cc: Or Gerlitz, Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F47171.4070003@solarflare.com>
On Thu, Mar 24, 2016 at 4:00 PM, Edward Cree <ecree@solarflare.com> wrote:
> On 24/03/16 21:50, Alexander Duyck wrote:
>> On Thu, Mar 24, 2016 at 1:17 PM, Edward Cree <ecree@solarflare.com> wrote:
>>> (Besides, I thought it was impossible for the partial checksum to be zero
>>> anyway because at least one of the inputs must be nonzero, and the end-
>>> around carry can never produce a zero. But maybe I'm getting confused here.)
>>
>> I forgot about that bit. I think you are right. We end up inverting
>> the output from csum fold so we are back to 0x1 - 0xFFFF as possible
>> values.
> Yes, it seems csum_fold inverts it and then the relevant callers invert it
> again. (I hadn't spotted either inversion, so I panicked for a moment when
> I thought it was getting inverted just once.)
>
>>> The headers should already be in cache, I thought, and this is only happening
>>> once per superframe. We're already going to have to crawl through the headers
>>> anyway to edit the lengths, I don't think it should cost much more to also
>>> inspect things like GRE csum bit or the UDP checksum field. And by
>>> identifying the 'next header' from this method as well, we don't need to add a
>>> new SKB_GSO_FOO bit or two every time we add another kind of encapsulation to
>>> the kernel.
>>
>> Right, but this gets back into the hardware flags issue. The whole
>> reason for SKB_GSO_FOO and SKB_GSO_FOO_CSUM is because hardware
>> typically supports one but not the other.
> Right, and for hardware supporting a specific GSO_FOO[_CSUM], we'll still
> need the flags - but in that case we'll be doing "old style" TSO anyway.
> GSO partial only really makes sense for a device that isn't (or at least can
> be prevented from) parsing the tunnel headers, and such hardware will support
> GSO_PARTIAL only, not a profusion of GSO_FOO (with or without _CSUM).
>
> So: in the initial transmit path we build a coherent superframe; when we get
> to the device, we say either "oh, device doesn't support offloads at all,
> call GSO", or "oh, device supports this particular GSO type for TSO, so give
> it the superframe SKB to do what it wants with", or "oh, device supports
> GSO_PARTIAL, let's call skb_mac_gso_partial [i.e. chain of new callbacks I
> was describing in previous email] and then give the non-coherent SKB to the
> device".
What you are proposing is essentially forking GSO. I really don't
like that. I prefer the approach we have now where essentially GSO
partial is something checked for at the very end and we have a few
checks when putting together the headers so we do the right thing.
> And in that last case it doesn't matter what the GSO type is, because the
> hardware can handle anything as long as we transform it into GSO partial.
Yes and no. The problem is trying to figure out what the device
supports. The hw_enc_features flags are only meant to be used with
one layer of tunnels. Are you suggesting we add some code to make
sure the GSO tunnel types are kept mutually exclusive and add an extra
offload that forces GSO/GSO_PARTIAL if they aren't?
This is sounding very complicated. I think we would be better off if
you took the time to try and implement some of this yourself so you
could see how feasible it is.
>> The other thing to keep in mind is it is much easier to figure out
>> what offloads we can make use of when we are only dealing with at most
>> 1 layer of tunnels. When we start getting into stacked tunnels it
>> really becomes impossible to figure out what features in the hardware
>> we can still make use of. You throw everything and the kitchen sink
>> on it and we have to give up and would be doing everything in
>> software. At least if we restrict the features being requested the
>> hardware has a chance to perhaps be useful.
> I think all the protocol-specific offloads can just say "don't even try
> if you've got stacked tunnels"; it's not something I expect current
> hardware to support, and if GSO partial works out then a hw vendor would
> have to be crazy to add such support in the future. But GSO partial
> doesn't care about stacked tunnels, it still works. Unless of course
> someone in the middle wants to change their IP IDs or GRE Counter or
> whatever, in which case they can mark the skb as "not gso_partial-safe"
> when they first build their header, or their gso_partial callback can
> return "nope", and then we fall back on a protocol-specific offload (if
> one's available, which it won't be for stacked tunnels) or software
> segmentation.
That is basically what happens now. When the tunnel gets configured
if there are any options that would prevent GSO from working then the
tunnel disables TSO support. That way the frames are segmented
further up the stack and then come through and are built around the
segmented frames.
>> For example one of the reasons why I know the outer UDP checksum works
>> the way it does with tunnel GSO is because the i40e and i40evf drivers
>> already have hardware that supports UDP_TUNNEL_CSUM. As such in order
>> to do what you are proposing we would likely have to rip that code out
>> and completely rewrite it since it would change out the checksum value
>> given to the device.
> I'm not sure I understand, so bear with me. AIUI, UDP_TUNNEL_CSUM means
> that the device will fill in both inner and outer L4 checksums. But it can
> only do that if it knows where the inner and outer L4 headers *are*, and I
> thought you were going to tell it not to treat it as a tunnel at all, but
> instead as a giant L3 header. So it won't touch the outer L4 checksum at
> all (and, in the case of stacked tunnels, it wouldn't touch the 'middle'
> one either - it only sees the innermost L4 header).
There is hardware that supports this already without GSO partial.
Specifically the i40e driver supports a XL722 device that supports
doing the outer UDP tunnel checksum in addition to the inner TCP
checksum. I'm suspecting there will be hardware from other vendors in
the near future that will have support as well.
The ixgbe and igb drivers will be taking the giant L3 header approach.
> So while the driver would need to change to add GSO_partial support, the
> hardware wouldn't.
> Where am I going wrong?
>
>> I'm not going to get into an argument over the merits of PMTU. The
>> fact is I don't believe it is worth the effort in order to enable the
>> tunnel-in-tunnel case you envision.
> I agree it's not worth the effort to implement it right now. But some day
> it will be, and at that point, if we've designed GSO partial the way I'm
> arguing for, it will just magically start working when we disable
> fragmentation on inner tunnels. And in the meantime we'll be using a more
> generic interface that hopefully won't encourage driver (and hardware)
> designers to see the gso_type enum as a list of protocols to support.
> Again, tunnel-in-tunnel isn't important in itself, but as a canary for
> "can we support whatever encapsulation protocol comes down the pike";
> arguing that you won't be able to set DF on your inner tunnel because
> operational consideration XYZ is missing the point IMHO.
It turns out that UDP tunnels seem to be the only real problem child
in terms of the DF bit. I was looking over the GRE code and it
already sets the DF bit and supports PMTU. Odds are we would probably
need to figure out how to borrow code from there and apply it to the
UDP tunnels.
>> Also another thing to keep in mind is you would have to find a way to
>> identify that a tunnel requesting to be segmented is setting the DF
>> bit in the outer headers.
> But that's easy, as above: either foo_xmit sets a "don't GSO-partial me"
> flag in the SKB, or foo_gso_partial returns -EMIGHTBEFRAGMENTED. The
> former is probably preferable for performance reasons.
Still ugly.
Honestly I don't know if it is worth all this trouble because we are
just as likely to run out of headroom which would probably be a more
expensive operation then giving up on the segmentation offloads and
only supporting HW_CSUM anyway.
>>>> On top of that it occurred to me that GRE cannot be screened in GRO
>>>> for the outer-IP-ID check. Basically what can happen is on devices
>>>> that don't parse inner headers for GRE we can end up with two TCP
>>>> flows from the same tunnel essentially stomping on each other and
>>>> causing one another to get evicted for having an outer IP-ID that
>>>> doesn't match up with the expected value.
>>
>>> Yes, that does seem problematic - you'd need to save away the IPID check
>>> result and only evict once you'd ascertained they were the same flow. All
>>> the more reason to try to make your tunnels use DF *grin*.
>>
>> Feel free to solve the problem if you believe it is that easy. There
>> is nothing in my patches to prevent that.
> I'm not saying it's easy. The GRO side of your patches look great to me
> and I think you're more likely to solve it than I am. But I will have a
> go at it too, if I have time.
My concern is that what you are talking about ends up being a pretty
significant change to the transmit path itself. Really the stack
doesn't want things segmented like that until you hit the GSO layer
and I think that might be where you run into issues.
> Though I must admit, I don't quite understand why we have to restrict to
> (incrementing or constant) in GRO, rather than just saying "it's DF, we
> can ignore the IP IDs and just aggregate it all together". We won't be
> producing the original IP IDs anyway if we resegment, because we don't
> know whether they were originally incrementing or constant. (In other
> words, "RFC6864 compliant GRO" as you've defined it can reverse either
> GSO or GSO partial, but GSO (in either form) can't always reverse
> RFC6864 compliant GRO.)
Actually for v2 of this GRO code I am taking the "it's DF just ignore
the IP ID entirely" approach. Basically IPv4 with DF set will behave
the same as IPv6 by setting flush_id to 0 instead of computing the
offset. I realized I was still deriving information from the IP ID
and RFC 6864 says we cannot do that for atomic datagrams.
- Alex
^ permalink raw reply
* [PATCH net] bpf: add missing map_flags to bpf_map_show_fdinfo
From: Daniel Borkmann @ 2016-03-24 23:30 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, netdev, Daniel Borkmann
Add map_flags attribute to bpf_map_show_fdinfo(), so that tools like
tc can check for them when loading objects from a pinned entry, e.g.
if user intent wrt allocation (BPF_F_NO_PREALLOC) is different to the
pinned object, it can bail out. Follow-up to 6c9059817432 ("bpf:
pre-allocate hash map elements"), so that tc can still support this
with v4.6.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/syscall.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2a2efe1..adc5e4b 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -137,11 +137,13 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)
"map_type:\t%u\n"
"key_size:\t%u\n"
"value_size:\t%u\n"
- "max_entries:\t%u\n",
+ "max_entries:\t%u\n"
+ "map_flags:\t%#x\n",
map->map_type,
map->key_size,
map->value_size,
- map->max_entries);
+ map->max_entries,
+ map->map_flags);
}
#endif
--
1.9.3
^ permalink raw reply related
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Edward Cree @ 2016-03-24 23:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: Or Gerlitz, Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <CAKgT0Uc083WTmag4_B5LkVvi+iDoFtEjB0Xx4u92-Wu5S=6mzw@mail.gmail.com>
On 24/03/16 21:50, Alexander Duyck wrote:
> On Thu, Mar 24, 2016 at 1:17 PM, Edward Cree <ecree@solarflare.com> wrote:
>> (Besides, I thought it was impossible for the partial checksum to be zero
>> anyway because at least one of the inputs must be nonzero, and the end-
>> around carry can never produce a zero. But maybe I'm getting confused here.)
>
> I forgot about that bit. I think you are right. We end up inverting
> the output from csum fold so we are back to 0x1 - 0xFFFF as possible
> values.
Yes, it seems csum_fold inverts it and then the relevant callers invert it
again. (I hadn't spotted either inversion, so I panicked for a moment when
I thought it was getting inverted just once.)
>> The headers should already be in cache, I thought, and this is only happening
>> once per superframe. We're already going to have to crawl through the headers
>> anyway to edit the lengths, I don't think it should cost much more to also
>> inspect things like GRE csum bit or the UDP checksum field. And by
>> identifying the 'next header' from this method as well, we don't need to add a
>> new SKB_GSO_FOO bit or two every time we add another kind of encapsulation to
>> the kernel.
>
> Right, but this gets back into the hardware flags issue. The whole
> reason for SKB_GSO_FOO and SKB_GSO_FOO_CSUM is because hardware
> typically supports one but not the other.
Right, and for hardware supporting a specific GSO_FOO[_CSUM], we'll still
need the flags - but in that case we'll be doing "old style" TSO anyway.
GSO partial only really makes sense for a device that isn't (or at least can
be prevented from) parsing the tunnel headers, and such hardware will support
GSO_PARTIAL only, not a profusion of GSO_FOO (with or without _CSUM).
So: in the initial transmit path we build a coherent superframe; when we get
to the device, we say either "oh, device doesn't support offloads at all,
call GSO", or "oh, device supports this particular GSO type for TSO, so give
it the superframe SKB to do what it wants with", or "oh, device supports
GSO_PARTIAL, let's call skb_mac_gso_partial [i.e. chain of new callbacks I
was describing in previous email] and then give the non-coherent SKB to the
device".
And in that last case it doesn't matter what the GSO type is, because the
hardware can handle anything as long as we transform it into GSO partial.
> The other thing to keep in mind is it is much easier to figure out
> what offloads we can make use of when we are only dealing with at most
> 1 layer of tunnels. When we start getting into stacked tunnels it
> really becomes impossible to figure out what features in the hardware
> we can still make use of. You throw everything and the kitchen sink
> on it and we have to give up and would be doing everything in
> software. At least if we restrict the features being requested the
> hardware has a chance to perhaps be useful.
I think all the protocol-specific offloads can just say "don't even try
if you've got stacked tunnels"; it's not something I expect current
hardware to support, and if GSO partial works out then a hw vendor would
have to be crazy to add such support in the future. But GSO partial
doesn't care about stacked tunnels, it still works. Unless of course
someone in the middle wants to change their IP IDs or GRE Counter or
whatever, in which case they can mark the skb as "not gso_partial-safe"
when they first build their header, or their gso_partial callback can
return "nope", and then we fall back on a protocol-specific offload (if
one's available, which it won't be for stacked tunnels) or software
segmentation.
>> (And remember that this is separate from - and doesn't impact - the existing
>> GSO code; this is a bunch of new foo_gso_partial() callbacks, distinct from
>> the foo_gso_segment() ones; and it's about preparing a superframe for hardware
>> offload rather than doing the segmentation in software. I think it's
>> preferable to have some preparation happen in software if that allows hardware
>> to be totally dumb. (Apologies if that was already all obvious.))
>
> This is where you and I differ. I think there are things you are overlooking.
>
> For example one of the reasons why I know the outer UDP checksum works
> the way it does with tunnel GSO is because the i40e and i40evf drivers
> already have hardware that supports UDP_TUNNEL_CSUM. As such in order
> to do what you are proposing we would likely have to rip that code out
> and completely rewrite it since it would change out the checksum value
> given to the device.
I'm not sure I understand, so bear with me. AIUI, UDP_TUNNEL_CSUM means
that the device will fill in both inner and outer L4 checksums. But it can
only do that if it knows where the inner and outer L4 headers *are*, and I
thought you were going to tell it not to treat it as a tunnel at all, but
instead as a giant L3 header. So it won't touch the outer L4 checksum at
all (and, in the case of stacked tunnels, it wouldn't touch the 'middle'
one either - it only sees the innermost L4 header).
So while the driver would need to change to add GSO_partial support, the
hardware wouldn't.
Where am I going wrong?
> I'm not going to get into an argument over the merits of PMTU. The
> fact is I don't believe it is worth the effort in order to enable the
> tunnel-in-tunnel case you envision.
I agree it's not worth the effort to implement it right now. But some day
it will be, and at that point, if we've designed GSO partial the way I'm
arguing for, it will just magically start working when we disable
fragmentation on inner tunnels. And in the meantime we'll be using a more
generic interface that hopefully won't encourage driver (and hardware)
designers to see the gso_type enum as a list of protocols to support.
Again, tunnel-in-tunnel isn't important in itself, but as a canary for
"can we support whatever encapsulation protocol comes down the pike";
arguing that you won't be able to set DF on your inner tunnel because
operational consideration XYZ is missing the point IMHO.
> Also another thing to keep in mind is you would have to find a way to
> identify that a tunnel requesting to be segmented is setting the DF
> bit in the outer headers.
But that's easy, as above: either foo_xmit sets a "don't GSO-partial me"
flag in the SKB, or foo_gso_partial returns -EMIGHTBEFRAGMENTED. The
former is probably preferable for performance reasons.
>>> On top of that it occurred to me that GRE cannot be screened in GRO
>>> for the outer-IP-ID check. Basically what can happen is on devices
>>> that don't parse inner headers for GRE we can end up with two TCP
>>> flows from the same tunnel essentially stomping on each other and
>>> causing one another to get evicted for having an outer IP-ID that
>>> doesn't match up with the expected value.
>
>> Yes, that does seem problematic - you'd need to save away the IPID check
>> result and only evict once you'd ascertained they were the same flow. All
>> the more reason to try to make your tunnels use DF *grin*.
>
> Feel free to solve the problem if you believe it is that easy. There
> is nothing in my patches to prevent that.
I'm not saying it's easy. The GRO side of your patches look great to me
and I think you're more likely to solve it than I am. But I will have a
go at it too, if I have time.
Though I must admit, I don't quite understand why we have to restrict to
(incrementing or constant) in GRO, rather than just saying "it's DF, we
can ignore the IP IDs and just aggregate it all together". We won't be
producing the original IP IDs anyway if we resegment, because we don't
know whether they were originally incrementing or constant. (In other
words, "RFC6864 compliant GRO" as you've defined it can reverse either
GSO or GSO partial, but GSO (in either form) can't always reverse
RFC6864 compliant GRO.)
-Ed
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-24 22:57 UTC (permalink / raw)
To: Vijay Pandurangan; +Cc: Cong Wang, Linux Kernel Network Developers, ej
In-Reply-To: <CAKUBDd91rR7QTwCO6L6ZfRe4fuHw0L5+Zi7qm0uF018dwVGCLg@mail.gmail.com>
On 03/24/2016 03:49 PM, Vijay Pandurangan wrote:
> Hmm that's troubling. We tested various UDP configurations but I don't think we tested this setup. Do you have code or more specific instructions we can use to
> replicate this bug?
The user-space app is not open source, and the routing setup is
a bit tricky as well.
I'm not certain it is specific to UDP, probably it is not.
I'm using sendmmsg to transmit frames on the packet socket, so
possibly something in that path is key.
Truth is, I can debug this with printk and see what is going
on if you have no immediate ideas what is going wrong.
Thanks,
Ben
>
> On Mar 24, 2016 6:02 PM, "Ben Greear" <greearb@candelatech.com <mailto:greearb@candelatech.com>> wrote:
>
> I have an application that creates two pairs of veth devices.
>
> a <-> b c <-> d
>
> b and c have a raw packet socket opened on them and I 'bridge' frames
> between b and c to provide network emulation (ie, configurable delay).
>
>
> I put IP 1.1.1.1/24 <http://1.1.1.1/24> on a, 1.1.1.2/24 <http://1.1.1.2/24> on d, and then create a UDP connection
> (using policy based routing to ensure frames are sent on the appropriate
> interfaces).
>
> This is user-space only app, and kernel in this case is completely unmodified.
>
> The commit below breaks this feature: UDP frames are sniffed on both a and d ports
> (in both directions), but the UDP socket does not receive frames.
>
> Using normal ethernet ports, this network emulation feature works fine, so it is
> specific to VETH.
>
> A similar test with just sending UDP between a single veth pair: e <-> f
> works fine. Maybe it has something to do with raw packets?
>
>
> The patch below is the culprit:
>
>
> [greearb@ben-dt3 linux-2.6]$ git bisect bad
> ce8c839b74e3017996fad4e1b7ba2e2625ede82f is the first bad commit
> commit ce8c839b74e3017996fad4e1b7ba2e2625ede82f
> Author: Vijay Pandurangan <vijayp@vijayp.ca <mailto:vijayp@vijayp.ca>>
> Date: Fri Dec 18 14:34:59 2015 -0500
>
> veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.
>
> Packets that arrive from real hardware devices have ip_summed ==
> CHECKSUM_UNNECESSARY if the hardware verified the checksums, or
> CHECKSUM_NONE if the packet is bad or it was unable to verify it. The
> current version of veth will replace CHECKSUM_NONE with
> CHECKSUM_UNNECESSARY, which causes corrupt packets routed from hardware to
> a veth device to be delivered to the application. This caused applications
> at Twitter to receive corrupt data when network hardware was corrupting
> packets.
>
> ...
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 0ef4a5a..ba21d07 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -117,12 +117,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
> kfree_skb(skb);
> goto drop;
> }
> - /* don't change ip_summed == CHECKSUM_PARTIAL, as that
> - * will cause bad checksum on forwarded packets
> - */
> - if (skb->ip_summed == CHECKSUM_NONE &&
> - rcv->features & NETIF_F_RXCSUM)
> - skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
> struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
>
>
> Any suggestions for how to fix this so that I get the old working behaviour and
> the bug this patch was trying to fix is also resolved?
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com <mailto:greearb@candelatech.com>>
> Candela Technologies Inc http://www.candelatech.com
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 22:49 UTC (permalink / raw)
To: Yann Ylavic
Cc: Linux Kernel Network Developers, Tom Herbert, Willy Tarreau,
Tolga Ceylan, Craig Gallek, Josh Snyder, Aaron Conole,
David S. Miller, Daniel Borkmann
In-Reply-To: <CAKQ1sVP_bbGYnVW=9WDnuzJ7qcC1xrM3qmeYFk-c3fi696F5-g@mail.gmail.com>
On Thu, 2016-03-24 at 23:40 +0100, Yann Ylavic wrote:
> FWIW, I find:
>
> const struct bpf_insn prog[] = {
> /* BPF_MOV64_REG(BPF_REG_6, BPF_REG_1) */
> { BPF_ALU64 | BPF_MOV | BPF_X, BPF_REG_6, BPF_REG_1, 0, 0 },
> /* BPF_LD_ABS(BPF_W, 0) R0 = (uint32_t)skb[0] */
> { BPF_LD | BPF_ABS | BPF_W, 0, 0, 0, 0 },
> /* BPF_ALU64_IMM(BPF_MOD, BPF_REG_0, mod) */
> { BPF_ALU64 | BPF_MOD | BPF_K, BPF_REG_0, 0, 0, mod },
> /* BPF_EXIT_INSN() */
> { BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
> };
> (and all the way to make it run)
>
> something quite unintuitive from a web server developper perspective,
> simply to make SO_REUSEPORT work with forked TCP listeners (probably
> as it should out of the box)...
That is why EBPF has LLVM backend.
Basically you can write your "BPF" program in C, and let llvm convert it
into EBPF.
Sure, you still can write BPF manually, as you could write HTTPS server
in assembly.
^ permalink raw reply
* [RFC PATCH] tcp: Add SOF_TIMESTAMPING_TX_EOR and allow MSG_EOR in tcp_sendmsg
From: Martin KaFai Lau @ 2016-03-24 22:46 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Eric Dumazet, Neal Cardwell, Willem de Bruijn,
Yuchung Cheng
This patch extends the SO_TIMESTAMPING work and the primary
objective is to track when TCP ACK is received for the
last byte of an application's message (e.g. HTTP2).
This patch allows the user process to use MSG_EOR during
tcp_sendmsg to tell the kernel that it is the last byte
of an application response message.
The user process can use the new SOF_TIMESTAMPING_TX_EOR to
ask the kernel to only track timestamp of the MSG_EOR byte.
Together with the existing SOF_TIMESTAMPING_TX_ACK and
SOF_TIMESTAMPING_OPT_ID, the user process knows which
response message the received TCP ACK is acknowledging.
The current SOF_TIMESTAMPING_TX_ACK is tracking the last
byte appended to a skb during the tcp_sendmsg. It may track
multiple bytes if the response spans across multiple skbs. While
it is enough to measure the response latency for application
protocol with a single request/response at a time (like HTTP 1.1
without pipeline), it does not work well for application protocol
with >1 pipeline responses (like HTTP2).
Each skb can only track one tskey (which is the seq number of
the last byte of the message). To allow tracking the
last byte of multiple response messages, this patch takes an approach
by not appending to the last skb during tcp_sendmsg if the last skb's
tskey will be overwritten. A similar case also happens during retransmit.
This approach avoids introducing another list to track the tskey. The
downside is that it will have less gso benefit and/or more outgoing
packets. Practically, due to the amount of measurement data generated,
sampling is usually used in production. (i.e. not every connection is
tracked).
One of our use case is at the webserver. The webserver tracks
the HTTP2 response latency by measuring when the webserver
sends the first byte to the socket till the TCP ACK of the last byte
is received. In the cases where we don't have client side
measurement, measuring from the server side is the only option.
In the cases we have the client side measurement, the server side
data can also be used to justify/cross-check-with the client
side data (e.g. is there slowness at the layer above the client's
TCP stack).
The TCP PRR paper [1] also measures a similar metrics:
"The TCP latency of a HTTP response when the server sends the first
byte until it receives the acknowledgment (ACK) for the last byte."
[1] Proportional Rate Reduction for TCP:
http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/37486.pdf
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
---
include/uapi/linux/net_tstamp.h | 3 ++-
net/ipv4/tcp.c | 23 ++++++++++++++++++-----
net/ipv4/tcp_output.c | 9 +++++++--
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 6d1abea..5376569 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -25,8 +25,9 @@ enum {
SOF_TIMESTAMPING_TX_ACK = (1<<9),
SOF_TIMESTAMPING_OPT_CMSG = (1<<10),
SOF_TIMESTAMPING_OPT_TSONLY = (1<<11),
+ SOF_TIMESTAMPING_TX_EOR = (1<<12),
- SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_TSONLY,
+ SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_EOR,
SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
SOF_TIMESTAMPING_LAST
};
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 08b8b96..7de96eb 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -428,11 +428,16 @@ void tcp_init_sock(struct sock *sk)
}
EXPORT_SYMBOL(tcp_init_sock);
-static void tcp_tx_timestamp(struct sock *sk, struct sk_buff *skb)
+static void tcp_tx_timestamp(struct sock *sk, struct sk_buff *skb, int flags)
{
if (sk->sk_tsflags) {
- struct skb_shared_info *shinfo = skb_shinfo(skb);
+ struct skb_shared_info *shinfo;
+ if ((sk->sk_tsflags & SOF_TIMESTAMPING_TX_EOR) &&
+ !(flags & MSG_EOR))
+ return;
+
+ shinfo = skb_shinfo(skb);
sock_tx_timestamp(sk, &shinfo->tx_flags);
if (shinfo->tx_flags & SKBTX_ANY_TSTAMP)
shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
@@ -957,7 +962,7 @@ new_segment:
offset += copy;
size -= copy;
if (!size) {
- tcp_tx_timestamp(sk, skb);
+ tcp_tx_timestamp(sk, skb, flags);
goto out;
}
@@ -1073,6 +1078,14 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
return err;
}
+static bool tcp_tx_ts_noappend_skb(const struct sock *sk,
+ const struct sk_buff *last_skb, int flags)
+{
+ return unlikely((sk->sk_tsflags & SOF_TIMESTAMPING_TX_EOR) &&
+ (flags & MSG_EOR) &&
+ (skb_shinfo(last_skb)->tx_flags & SKBTX_ANY_TSTAMP));
+}
+
int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -1144,7 +1157,7 @@ int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
copy = max - skb->len;
}
- if (copy <= 0) {
+ if (copy <= 0 || tcp_tx_ts_noappend_skb(sk, skb, flags)) {
new_segment:
/* Allocate new segment. If the interface is SG,
* allocate skb fitting to single page.
@@ -1237,7 +1250,7 @@ new_segment:
copied += copy;
if (!msg_data_left(msg)) {
- tcp_tx_timestamp(sk, skb);
+ tcp_tx_timestamp(sk, skb, flags);
goto out;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7d2dc01..ee415cb 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2488,7 +2488,8 @@ static void tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
}
/* Check if coalescing SKBs is legal. */
-static bool tcp_can_collapse(const struct sock *sk, const struct sk_buff *skb)
+static bool tcp_can_collapse(const struct sock *sk, const struct sk_buff *skb,
+ const struct sk_buff *to)
{
if (tcp_skb_pcount(skb) > 1)
return false;
@@ -2502,6 +2503,10 @@ static bool tcp_can_collapse(const struct sock *sk, const struct sk_buff *skb)
/* Some heurestics for collapsing over SACK'd could be invented */
if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)
return false;
+ if (unlikely((sk->sk_tsflags & SOF_TIMESTAMPING_TX_EOR) &&
+ (skb_shinfo(to)->tx_flags & SKBTX_ANY_TSTAMP) &&
+ (skb_shinfo(skb)->tx_flags & SKBTX_ANY_TSTAMP)))
+ return false;
return true;
}
@@ -2522,7 +2527,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
return;
tcp_for_write_queue_from_safe(skb, tmp, sk) {
- if (!tcp_can_collapse(sk, skb))
+ if (!tcp_can_collapse(sk, skb, to))
break;
space -= skb->len;
--
2.5.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Yann Ylavic @ 2016-03-24 22:40 UTC (permalink / raw)
To: Linux Kernel Network Developers
Cc: Tom Herbert, Eric Dumazet, Willy Tarreau, Tolga Ceylan,
Craig Gallek, Josh Snyder, Aaron Conole, David S. Miller,
Daniel Borkmann
In-Reply-To: <56F42A00.7050002@iogearbox.net>
On Thu, Mar 24, 2016 at 6:55 PM, Daniel Borkmann wrote:
> On 03/24/2016 06:26 PM, Tom Herbert wrote:
>>
>> On Thu, Mar 24, 2016 at 10:01 AM, Eric Dumazet wrote:
>>>
>>> Really, when BPF can be the solution, we wont allow adding new stuff in
>>> the kernel in the old way.
>>
>> I completely agree with this, but I wonder if we now need a repository
>> of useful BPF modules. So in the case of implementing functionality
>> like in SO_REUSEPORT_LISTEN_OFF that might just become a common BPF
>> program we could direct people to use.
>
> Good point. There's tools/testing/selftests/net/ containing already
> reuseport
> BPF example, maybe it could be extended.
FWIW, I find:
const struct bpf_insn prog[] = {
/* BPF_MOV64_REG(BPF_REG_6, BPF_REG_1) */
{ BPF_ALU64 | BPF_MOV | BPF_X, BPF_REG_6, BPF_REG_1, 0, 0 },
/* BPF_LD_ABS(BPF_W, 0) R0 = (uint32_t)skb[0] */
{ BPF_LD | BPF_ABS | BPF_W, 0, 0, 0, 0 },
/* BPF_ALU64_IMM(BPF_MOD, BPF_REG_0, mod) */
{ BPF_ALU64 | BPF_MOD | BPF_K, BPF_REG_0, 0, 0, mod },
/* BPF_EXIT_INSN() */
{ BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
};
(and all the way to make it run)
something quite unintuitive from a web server developper perspective,
simply to make SO_REUSEPORT work with forked TCP listeners (probably
as it should out of the box)...
Regards,
Yann.
^ permalink raw reply
* RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Florian Fainelli @ 2016-03-24 22:36 UTC (permalink / raw)
To: Bryan.Whitehead, andrew; +Cc: davem, netdev
In-Reply-To: <90A7E81AE28BAE4CBDDB3B35F187D264402F31B5@CHN-SV-EXMX02.mchp-main.com>
On March 24, 2016 2:16:30 PM PDT, Bryan.Whitehead@microchip.com wrote:
>> -----Original Message-----
>Hi Andrew,
>
>Sorry to bother you with this. But I'm having major difficulty with
>getting my dsa driver entry points called.
>
>Here is what I've done so far.
>
>I copied drivers/net/dsa/mv88x6060.c into
>drivers/net/dsa/mchp9352_dsa.c
>I then modified mchp9352_dsa.c as follows
>I emptied out the function bodies, and replaced them with a printk("Not
>Implemented\n");
>
>I did the same thing with net/dsa/tag_trailer.c which was copied into
>net/dsa/tag_mchp9352.c
>And function bodies were replaced with printk("Not Implemented\n");
>
>I also modified net/dsa/dsa.c, and net/dsa/slave.c, to include the
>hooks into my new tag files.
>
>My intent was to just see one of my "Not Implemented" functions called,
>and then I would focus on implementing it.
>
>But so far I have not seen any of my "Not Implemented" functions
>called.
>
>Here is what I know so far.
>
>It appears the dsa.c is not able to attach my underlying net device.
>And that seems to be due to it is unable to find the mdio_bus.
>So I modified my net device driver so that in probe it calls
>of_mdiobus_register instead of mdiobus_register.
>And of_mdiobus_register seems to be looking for some kind of phy
>definitions in the device tree, which it does not find. And so it does
>not appear to register the bus in such a way that dsa.c can connect to
>it.
If the lan9352 HW has a MDIO bus, which connects to internal switch, or PHYs from the switch, there must be an mii_bus structure allocated and registered to make that available.
>
>So the problem appears to be an issue with my device tree, which is
>partially shown below.
>
>&gpmc {
> status = "okay";
> ranges = <0 0 0x10000000 0x08000000>; // CS0: 128M
> pinctrl-names = "default";
> pinctrl-0 = <&gpmc_pins>;
> lan9352: ethernet@gpmc {
> compatible = "microchip,lan9352";
> interrupt-parent = <&gpio0>;
> interrupts = <7 8>;//7==GPIO bit 7, 8 = Active low level triggered.
>
> bank-width = <2>;
>
> phy-mode = "mii";
Having a phy-mode without a phy-handle property or fixed-phy subnode does not sound correct nor useful.
Considering that you are interfaced to a built-in switch a fixed-link subnode is needed to tell the Ethernet MAC about the link parameters. See drivers/net/ethernet/broadcom/bcmsysport.c for an example by grepping for fixed_phy.
>
> reg = <0 0 0x10000>;
>
> reg-io-width = <4>;
> microchip,save-mac-address;
> microchip,irq-push-pull;
>
> };
>};
>
>/ {
> dsa@0 {
> compatible = "microchip,dsa";
Did you also update net/dsa/dsa.c so it tries to match this compatible string? DSA is right now a platform device (though there is pending working from Andrew to change that) so we need a way to probe DSA.
> #address-cells = <2>;
> #size-cells = <0>;
> dsa,ethernet = <&lan9352>;
That looks correct.
> dsa,mii-bus = <&lan9352>;
But not that one, this must point to a device tree node which is a MDIO bus controller. It could be a sub-node to lan9352 provided that there is a way to expose a MDIO bus controller.
Moving forward, with Andrew's latest patches included, this may become entirely optional, but it is not just now.
--
Florian
^ permalink raw reply
* Re: [PATCH net] net: ipv4: Multipath needs to handle unreachable nexthops
From: Julian Anastasov @ 2016-03-24 22:33 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <1458833154-39091-1-git-send-email-dsa@cumulusnetworks.com>
Hello,
On Thu, 24 Mar 2016, David Ahern wrote:
> Multipath route lookups should consider knowledge about next hops and not
> select a hop that is known to be failed.
...
> The failed path can be avoided by considering known neighbor information
> when selecting next hops. If the neighbor lookups fails we have no
> knowledge about the nexthop, so give it a shot. If there is an entry
> then only select the nexthop if the state is sane. This is similar to
> what fib_detect_death does for some single path cases.
fib_detect_death works with alternatives and
the neighbor status is used as the only present info
in kernel.
But for multipath routes we can also consider the
nexthops as "alternatives", so it depends on how one uses
the multipath mechanism. The ability to fallback to
another nexthop assumes one connection is allowed to
move from one ISP to another. What if the second ISP
decides to reject the connection? What we have is a
broken connection just because the retransmits
were diverted to wrong place in the hurry. So, the
nexthops can be compatible or incompatible. For your
setup they are, for others they are not.
For multipath setups I recall for the following
possible cases:
1. Every packet from connection hits multipath route
and what we want is the connection to use only one
nexthop. If nexthop fails then the connection fails.
Latest changes for the multipath algorithm chose this
option: use hash to map traffic to nexthop and any
fallbacks to another nexthop are not allowed because
we should follow the hash mapping. Works when nexthops
use different ISPs.
2. Only the first packet hits multipath route. With
the help from CONNMARK all next packets from connection
use the same nexthop by using another unicast route.
The goal here is the multipath route to be used just to
balance connections. This works best when the nexthop
selection was random and not a hash based. This was
how the previous algorithm worked. Fallbacks are
desired because it is fine to select alive nexthop
for the first packet, but wrong for the next packets.
My preference was for the random selection,
I don't know why we restricted the new algorithm. May be
because in the common case when just a single default
multipath route is created we want to use all nexthops
in a ideal world where all nexthops are alive.
So, if the kernel used a random selection
your fallback algorithm should help. But it is fragile
for the simple setup with single default multipath route.
May be what we miss is the ability to choose between
random and hash-based selection. Then your patch may be
useful but only for setup 2 (multipath route hit only by
first packet). So, your patch may come with a sysctl var
that explains your current patch logic: "avoid failed nexthops,
never probe them, wait their failed entry to be expired by
neigh_periodic_work and just then we can use the nexthop
by creating new entry to probe the GW". Who will trigger
probes often enough to maintain fresh state?
Also, one may argue that such decisions should
be done in user space. It is common the direct routers
to answer ARP even while their uplink is down. In
the common case, one may need to ping 2-3 indirect
gateways to decide if a path is alive and then to recreate
the default multipath route.
More comments below...
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index d97268e8ff10..28fc6700c2b1 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1563,13 +1563,43 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
> void fib_select_multipath(struct fib_result *res, int hash)
> {
> struct fib_info *fi = res->fi;
> + struct neighbour *n;
> + int state;
>
> for_nexthops(fi) {
> if (hash > atomic_read(&nh->nh_upper_bound))
> continue;
>
> - res->nh_sel = nhsel;
> - return;
> + state = NUD_NONE;
> + n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
Sometimes nh_gw is a local IP, you can call
neigh_lookup (or a lockless RCU-BH variant) only for the
nh_scope == RT_SCOPE_LINK case, just like in fib_select_default.
> + if (n) {
> + state = n->nud_state;
> + neigh_release(n);
> + }
> + if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
NUD_REACHABLE is part of NUD_VALID, so this is shorter:
if (!n || (state & NUD_VALID)) {
> + res->nh_sel = nhsel;
> + return;
> + }
> + } endfor_nexthops(fi);
> +
> + /* try the nexthops again, but covering the entries
> + * skipped by the hash
> + */
> + fi = res->fi;
> + for_nexthops(fi) {
> + if (hash <= atomic_read(&nh->nh_upper_bound))
This is dangerous, we can try a RTNH_F_DEAD entry
with nh_upper_bound = -1. This can work with 2 checks:
if (hash <= atomic_read(&nh->nh_upper_bound))
break;
if (atomic_read(&nh->nh_upper_bound) < 0)
continue;
> + continue;
> +
> + state = NUD_NONE;
> + n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
> + if (n) {
> + state = n->nud_state;
> + neigh_release(n);
> + }
> + if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
> + res->nh_sel = nhsel;
> + return;
> + }
> } endfor_nexthops(fi);
If all are failed why not use the nexthop that was
selected by the hash? Even if it is failed, new ARP probe
can succeed.
> /* Race condition: route has just become dead. */
> --
> 1.9.1
Regards
^ permalink raw reply
* Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Andrew Lunn @ 2016-03-24 22:06 UTC (permalink / raw)
To: Bryan.Whitehead; +Cc: davem, netdev
In-Reply-To: <90A7E81AE28BAE4CBDDB3B35F187D264402F31B5@CHN-SV-EXMX02.mchp-main.com>
> It appears the dsa.c is not able to attach my underlying net
> device. And that seems to be due to it is unable to find the
> mdio_bus.
> So I modified my net device driver so that in probe it calls
> of_mdiobus_register instead of mdiobus_register.
> And of_mdiobus_register seems to be looking for some kind of phy
> definitions in the device tree, which it does not find. And so it
> does not appear to register the bus in such a way that dsa.c can
> connect to it.
Hi Bryan
Are the sources for the ethernet driver available? I don't see them in
net-next.
There are two common ways for this to work, depending on the driver
architecture. Marvell devices have a separate mdio driver. In
kirkwood.dtsi you see:
mdio: mdio-bus@72004 {
compatible = "marvell,orion-mdio";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x72004 0x84>;
interrupts = <46>;
clocks = <&gate_clk 0>;
status = "disabled";
/* add phy nodes in board file */
};
and mvmdio.c calls of_mdiobus_register() passing this device node.
The other way is that the mdio is part of the ethernet
driver. e.g. for the Freescale FEC:
&fec1 {
phy-mode = "rmii";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fec1>;
status = "okay";
mdio0: mdio {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
};
};
In this case, of_mdiobus_register() is passed the mdio0 device node.
> &gpmc {
> status = "okay";
> ranges = <0 0 0x10000000 0x08000000>; // CS0: 128M
> pinctrl-names = "default";
> pinctrl-0 = <&gpmc_pins>;
> lan9352: ethernet@gpmc {
> compatible = "microchip,lan9352";
> interrupt-parent = <&gpio0>;
> interrupts = <7 8>;//7==GPIO bit 7, 8 = Active low level triggered.
>
> bank-width = <2>;
>
> phy-mode = "mii";
>
> reg = <0 0 0x10000>;
>
> reg-io-width = <4>;
> microchip,save-mac-address;
> microchip,irq-push-pull;
So i expect to see something like this here:
mdio0: mdio {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
};
> };
> };
>
> / {
> dsa@0 {
> compatible = "microchip,dsa";
> #address-cells = <2>;
> #size-cells = <0>;
> dsa,ethernet = <&lan9352>;
> dsa,mii-bus = <&lan9352>;
and this would be
dsa,mii-bus = <&mdio0>;
> switch@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0 0>; /* MDIO address 0, switch 0 in tree */
> port@0 {
> reg = <0>;
> label = "cpu";
> };
> port@1 {
> reg = <1>;
> label = "lan1";
> };
> port@2 {
> reg = <2>;
> label = "lan2";
> };
> };
> };
> };
Andrew
^ 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