* Re: [PATCH net-next 02/10] udp: add gso
From: Alexander Duyck @ 2018-04-25 20:38 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Netdev, David Miller, dmichail, Willem de Bruijn
In-Reply-To: <20180425185601.55511-3-willemdebruijn.kernel@gmail.com>
On Wed, Apr 25, 2018 at 11:55 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Implement generic segmentation offload for udp datagrams. Callers can
> concatenate and send at once the payload of multiple datagrams with
> the same destination.
>
> To set segment size, the caller sets socket option UDP_SEGMENT to the
> length of each discrete payload. This value must be smaller than or
> equal to the relevant MTU.
>
> A follow-up patch adds cmsg UDP_SEGMENT to specify segment size on a
> per send call basis.
>
> Total byte length may then exceed MTU. If not an exact multiple of
> segment size, the last segment will be shorter.
>
> UDP GSO is not UFO. UFO fragments a single large datagram. GSO splits
> a large payload into a number of discrete UDP datagrams.
>
> The implementation adds a GSO type SKB_UDP_GSO_L4 to differentiate it
> from UFO (SKB_UDP_GSO). It adds a gso_size field to the udp socket,
> ip(v6) cmsg cookie and inet_cork structure to be able to set the value
> at setsockopt or cmsg time and to work with both lockless and corked
> paths.
>
> IPPROTO_UDPLITE is excluded, as that protocol has no gso handler
> registered.
>
> Initial benchmark numbers show UDP GSO about as expensive as TCP GSO.
>
> tcp tso
> 3197 MB/s 54232 msg/s 54232 calls/s
> 6,457,754,262 cycles
>
> tcp gso
> 1765 MB/s 29939 msg/s 29939 calls/s
> 11,203,021,806 cycles
>
> tcp without tso/gso *
> 739 MB/s 12548 msg/s 12548 calls/s
> 11,205,483,630 cycles
>
> udp
> 876 MB/s 14873 msg/s 624666 calls/s
> 11,205,777,429 cycles
>
> udp gso
> 2139 MB/s 36282 msg/s 36282 calls/s
> 11,204,374,561 cycles
>
> [*] after reverting commit 0a6b2a1dc2a2
> ("tcp: switch to GSO being always on")
>
> Measured total system cycles ('-a') for one core while pinning both
> the network receive path and benchmark process to that core:
>
> perf stat -a -C 12 -e cycles \
> ./udpgso_bench_tx -C 12 -4 -D "$DST" -l 4
>
> Note the reduction in calls/s with GSO. Bytes per syscall drops
> increases from 1470 to 61818.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
It might be nice if you could break this into two patches. One for
actually doing the GSO in software, and another enabling the stack to
request it.
> ---
> include/linux/skbuff.h | 2 ++
> include/linux/udp.h | 3 +++
> include/net/inet_sock.h | 1 +
> include/net/ip.h | 1 +
> include/net/ipv6.h | 1 +
> include/net/udp.h | 4 ++++
> include/uapi/linux/udp.h | 1 +
> net/core/skbuff.c | 2 ++
> net/ipv4/ip_output.c | 9 ++++---
> net/ipv4/udp.c | 33 ++++++++++++++++++++++---
> net/ipv4/udp_offload.c | 52 +++++++++++++++++++++++++++++++++++++++-
> net/ipv6/ip6_offload.c | 6 +++--
> net/ipv6/ip6_output.c | 6 +++--
> net/ipv6/udp.c | 23 +++++++++++++++---
> net/ipv6/udp_offload.c | 19 ++++++++++++++-
> 15 files changed, 148 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index d274059529eb..a4a5c0c5cba8 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -573,6 +573,8 @@ enum {
> SKB_GSO_ESP = 1 << 15,
>
> SKB_GSO_UDP = 1 << 16,
> +
> + SKB_GSO_UDP_L4 = 1 << 17,
> };
>
Part of me really wishes we could just rename SKB_GSO_UDP to something
like SKB_GFO_UDP since GSO implies "segmentation" but what we really
mean is "fragmentation" in the case of the existing UDP offload.
> #if BITS_PER_LONG > 32
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index eaea63bc79bb..ca840345571b 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -55,6 +55,7 @@ struct udp_sock {
> * when the socket is uncorked.
> */
> __u16 len; /* total length of pending frames */
> + __u16 gso_size;
> /*
> * Fields specific to UDP-Lite.
> */
> @@ -87,6 +88,8 @@ struct udp_sock {
> int forward_deficit;
> };
>
> +#define UDP_MAX_SEGMENTS (1 << 6UL)
> +
> static inline struct udp_sock *udp_sk(const struct sock *sk)
> {
> return (struct udp_sock *)sk;
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 0a671c32d6b9..83d5b3c2ac42 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -147,6 +147,7 @@ struct inet_cork {
> __u8 ttl;
> __s16 tos;
> char priority;
> + __u16 gso_size;
> };
>
> struct inet_cork_full {
> diff --git a/include/net/ip.h b/include/net/ip.h
> index 7ec543a64bbc..bada1f1f871e 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -76,6 +76,7 @@ struct ipcm_cookie {
> __u8 ttl;
> __s16 tos;
> char priority;
> + __u16 gso_size;
> };
>
> #define IPCB(skb) ((struct inet_skb_parm*)((skb)->cb))
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 0dd722cab037..0a872a7c33c8 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -298,6 +298,7 @@ struct ipcm6_cookie {
> __s16 tclass;
> __s8 dontfrag;
> struct ipv6_txoptions *opt;
> + __u16 gso_size;
> };
>
> static inline struct ipv6_txoptions *txopt_get(const struct ipv6_pinfo *np)
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 0676b272f6ac..741d888d0fdb 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -174,6 +174,10 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
> struct udphdr *uh, udp_lookup_t lookup);
> int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>
> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features,
> + unsigned int mss, __sum16 check);
> +
> static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
> {
> struct udphdr *uh;
> diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
> index efb7b5991c2f..09d00f8c442b 100644
> --- a/include/uapi/linux/udp.h
> +++ b/include/uapi/linux/udp.h
> @@ -32,6 +32,7 @@ struct udphdr {
> #define UDP_ENCAP 100 /* Set the socket to accept encapsulated packets */
> #define UDP_NO_CHECK6_TX 101 /* Disable sending checksum for UDP6X */
> #define UDP_NO_CHECK6_RX 102 /* Disable accpeting checksum for UDP6 */
> +#define UDP_SEGMENT 103 /* Set GSO segmentation size */
>
> /* UDP encapsulation types */
> #define UDP_ENCAP_ESPINUDP_NON_IKE 1 /* draft-ietf-ipsec-nat-t-ike-00/01 */
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index ff49e352deea..c647cfe114e0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4940,6 +4940,8 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> thlen = tcp_hdrlen(skb);
> } else if (unlikely(skb_is_gso_sctp(skb))) {
> thlen = sizeof(struct sctphdr);
> + } else if (shinfo->gso_type & SKB_GSO_UDP_L4) {
> + thlen = sizeof(struct udphdr);
> }
> /* UFO sets gso_size to the size of the fragmentation
> * payload, i.e. the size of the L4 (UDP) header is already
It might make more sense to look at converting this over to a switch
statement based off of shinfo(skb)->gso_type & GSO_TRANSPORT_MASK,
where the transport mask consists of the 4 bits that are supported.
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 2883ff1e909c..da4abbee10f7 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -882,7 +882,8 @@ static int __ip_append_data(struct sock *sk,
> skb = skb_peek_tail(queue);
>
> exthdrlen = !skb ? rt->dst.header_len : 0;
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP_MAX_MTU : cork->fragsize;
> +
> if (cork->tx_flags & SKBTX_ANY_SW_TSTAMP &&
> sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)
> tskey = sk->sk_tskey++;
> @@ -906,7 +907,7 @@ static int __ip_append_data(struct sock *sk,
> if (transhdrlen &&
> length + fragheaderlen <= mtu &&
> rt->dst.dev->features & (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM) &&
> - !(flags & MSG_MORE) &&
> + (!(flags & MSG_MORE) || cork->gso_size) &&
> !exthdrlen)
> csummode = CHECKSUM_PARTIAL;
>
> @@ -1135,6 +1136,8 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
> *rtp = NULL;
> cork->fragsize = ip_sk_use_pmtu(sk) ?
> dst_mtu(&rt->dst) : rt->dst.dev->mtu;
> +
> + cork->gso_size = sk->sk_type == SOCK_DGRAM ? ipc->gso_size : 0;
> cork->dst = &rt->dst;
> cork->length = 0;
> cork->ttl = ipc->ttl;
> @@ -1214,7 +1217,7 @@ ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
> return -EOPNOTSUPP;
>
> hh_len = LL_RESERVED_SPACE(rt->dst.dev);
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP_MAX_MTU : cork->fragsize;
>
> fragheaderlen = sizeof(struct iphdr) + (opt ? opt->optlen : 0);
> maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 6b9d8017b319..bda022c5480b 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -757,7 +757,8 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
> }
> EXPORT_SYMBOL(udp_set_csum);
>
> -static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4)
> +static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
> + struct inet_cork *cork)
> {
> struct sock *sk = skb->sk;
> struct inet_sock *inet = inet_sk(sk);
> @@ -777,6 +778,21 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4)
> uh->len = htons(len);
> uh->check = 0;
>
> + if (cork->gso_size) {
> + const int hlen = skb_network_header_len(skb) +
> + sizeof(struct udphdr);
> +
> + if (hlen + cork->gso_size > cork->fragsize)
> + return -EINVAL;
> + if (skb->len > cork->gso_size * UDP_MAX_SEGMENTS)
> + return -EINVAL;
> + if (skb->ip_summed != CHECKSUM_PARTIAL || is_udplite)
> + return -EIO;
> +
> + skb_shinfo(skb)->gso_size = cork->gso_size;
> + skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> + }
> +
> if (is_udplite) /* UDP-Lite */
> csum = udplite_csum(skb);
>
> @@ -828,7 +844,7 @@ int udp_push_pending_frames(struct sock *sk)
> if (!skb)
> goto out;
>
> - err = udp_send_skb(skb, fl4);
> + err = udp_send_skb(skb, fl4, &inet->cork.base);
>
> out:
> up->len = 0;
> @@ -922,6 +938,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> ipc.sockc.tsflags = sk->sk_tsflags;
> ipc.addr = inet->inet_saddr;
> ipc.oif = sk->sk_bound_dev_if;
> + ipc.gso_size = up->gso_size;
>
> if (msg->msg_controllen) {
> err = ip_cmsg_send(sk, msg, &ipc, sk->sk_family == AF_INET6);
> @@ -1037,7 +1054,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> &cork, msg->msg_flags);
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> - err = udp_send_skb(skb, fl4);
> + err = udp_send_skb(skb, fl4, &cork);
> goto out;
> }
>
> @@ -2367,6 +2384,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
> up->no_check6_rx = valbool;
> break;
>
> + case UDP_SEGMENT:
> + if (val < 0 || val > USHRT_MAX)
> + return -EINVAL;
> + up->gso_size = val;
> + break;
> +
> /*
> * UDP-Lite's partial checksum coverage (RFC 3828).
> */
> @@ -2457,6 +2480,10 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
> val = up->no_check6_rx;
> break;
>
> + case UDP_SEGMENT:
> + val = up->gso_size;
> + break;
> +
> /* The following two cannot be changed on UDP sockets, the return is
> * always 0 (which corresponds to the full checksum coverage of UDP). */
> case UDPLITE_SEND_CSCOV:
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index ea6e6e7df0ee..a3244768e45f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -187,6 +187,53 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
> }
> EXPORT_SYMBOL(skb_udp_tunnel_segment);
>
> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features,
> + unsigned int mss, __sum16 check)
> +{
> + struct udphdr *uh = udp_hdr(gso_skb);
> + struct sk_buff *segs;
> + unsigned int hdrlen;
> +
> + if (gso_skb->len <= sizeof(*uh) + mss)
> + return ERR_PTR(-EINVAL);
> +
> + uh->len = htons(sizeof(*uh) + mss);
> + uh->check = check;
> + skb_pull(gso_skb, sizeof(*uh));
> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
Normally I don't believe we modify the headers before calling
skb_segment. Normally that happens after via a while (skb->next) {}
type loop.
That way for things like GSO_PARTIAL we can update after segmentation
since there are only going to be 2 segments most likely instead of
multiple MSS sized segments.
> +
> + segs = skb_segment(gso_skb, features);
> + if (unlikely(IS_ERR_OR_NULL(segs)))
> + return segs;
> +
> + /* If last packet is not full, fix up its header */
> + if (segs->prev->len != hdrlen + mss) {
> + unsigned int mss_last = segs->prev->len - hdrlen;
> +
> + uh = udp_hdr(segs->prev);
> + uh->len = htons(sizeof(*uh) + mss_last);
> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
> + }
> +
You could probably just assume that this last segment is always going
to need to be updated regardless. If you are doing any sort of
segmentation the last segment will always need the length and checksum
updated anyway, and since you probably need to move over to the "while
(skb->next)" style loop then you could just assume the last segment
needs updating.
> + return segs;
> +}
> +
> +static struct sk_buff *__udp4_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features)
> +{
> + const struct iphdr *iph = ip_hdr(gso_skb);
> + unsigned int mss = skb_shinfo(gso_skb)->gso_size;
> +
> + if (!can_checksum_protocol(features, htons(ETH_P_IP)))
> + return ERR_PTR(-EIO);
> +
> + return __udp_gso_segment(gso_skb, features, mss,
> + udp_v4_check(sizeof(struct udphdr) + mss,
> + iph->saddr, iph->daddr, 0));
> +}
> +EXPORT_SYMBOL_GPL(__udp4_gso_segment);
> +
> static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
> netdev_features_t features)
> {
> @@ -203,12 +250,15 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
> goto out;
> }
>
> - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP))
> + if (!(skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP_L4)))
> goto out;
>
> if (!pskb_may_pull(skb, sizeof(struct udphdr)))
> goto out;
>
> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> + return __udp4_gso_segment(skb, features);
> +
> mss = skb_shinfo(skb)->gso_size;
> if (unlikely(skb->len <= mss))
> goto out;
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 4a87f9428ca5..5b3f2f89ef41 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -88,9 +88,11 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>
> if (skb->encapsulation &&
> skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
> - udpfrag = proto == IPPROTO_UDP && encap;
> + udpfrag = proto == IPPROTO_UDP && encap &&
> + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> else
> - udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
> + udpfrag = proto == IPPROTO_UDP && !skb->encapsulation &&
> + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
>
> ops = rcu_dereference(inet6_offloads[proto]);
> if (likely(ops && ops->callbacks.gso_segment)) {
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fa1db447405..a1c4a78132d2 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1240,6 +1240,8 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
> if (mtu < IPV6_MIN_MTU)
> return -EINVAL;
> cork->base.fragsize = mtu;
> + cork->base.gso_size = sk->sk_type == SOCK_DGRAM ? ipc6->gso_size : 0;
> +
> if (dst_allfrag(xfrm_dst_path(&rt->dst)))
> cork->base.flags |= IPCORK_ALLFRAG;
> cork->base.length = 0;
> @@ -1281,7 +1283,7 @@ static int __ip6_append_data(struct sock *sk,
> dst_exthdrlen = rt->dst.header_len - rt->rt6i_nfheader_len;
> }
>
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP6_MAX_MTU : cork->fragsize;
> orig_mtu = mtu;
>
> hh_len = LL_RESERVED_SPACE(rt->dst.dev);
> @@ -1329,7 +1331,7 @@ static int __ip6_append_data(struct sock *sk,
> if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
> headersize == sizeof(struct ipv6hdr) &&
> length <= mtu - headersize &&
> - !(flags & MSG_MORE) &&
> + (!(flags & MSG_MORE) || cork->gso_size) &&
> rt->dst.dev->features & (NETIF_F_IPV6_CSUM | NETIF_F_HW_CSUM))
> csummode = CHECKSUM_PARTIAL;
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 824797f8d1ab..86b7dd58d4b4 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1023,7 +1023,8 @@ static void udp6_hwcsum_outgoing(struct sock *sk, struct sk_buff *skb,
> * Sending
> */
>
> -static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6)
> +static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
> + struct inet_cork *cork)
> {
> struct sock *sk = skb->sk;
> struct udphdr *uh;
> @@ -1042,6 +1043,21 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6)
> uh->len = htons(len);
> uh->check = 0;
>
> + if (cork->gso_size) {
> + const int hlen = skb_network_header_len(skb) +
> + sizeof(struct udphdr);
> +
> + if (hlen + cork->gso_size > cork->fragsize)
> + return -EINVAL;
> + if (skb->len > cork->gso_size * UDP_MAX_SEGMENTS)
> + return -EINVAL;
> + if (skb->ip_summed != CHECKSUM_PARTIAL || is_udplite)
> + return -EIO;
> +
> + skb_shinfo(skb)->gso_size = cork->gso_size;
> + skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> + }
> +
> if (is_udplite)
> csum = udplite_csum(skb);
> else if (udp_sk(sk)->no_check6_tx) { /* UDP csum disabled */
> @@ -1093,7 +1109,7 @@ static int udp_v6_push_pending_frames(struct sock *sk)
> if (!skb)
> goto out;
>
> - err = udp_v6_send_skb(skb, &fl6);
> + err = udp_v6_send_skb(skb, &fl6, &inet_sk(sk)->cork.base);
>
> out:
> up->len = 0;
> @@ -1127,6 +1143,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> ipc6.hlimit = -1;
> ipc6.tclass = -1;
> ipc6.dontfrag = -1;
> + ipc6.gso_size = up->gso_size;
> sockc.tsflags = sk->sk_tsflags;
>
> /* destination address check */
> @@ -1333,7 +1350,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> msg->msg_flags, &cork, &sockc);
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> - err = udp_v6_send_skb(skb, &fl6);
> + err = udp_v6_send_skb(skb, &fl6, &cork.base);
> goto out;
> }
>
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index 2a04dc9c781b..f7b85b1e6b3e 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -17,6 +17,20 @@
> #include <net/ip6_checksum.h>
> #include "ip6_offload.h"
>
> +static struct sk_buff *__udp6_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features)
> +{
> + const struct ipv6hdr *ip6h = ipv6_hdr(gso_skb);
> + unsigned int mss = skb_shinfo(gso_skb)->gso_size;
> +
> + if (!can_checksum_protocol(features, htons(ETH_P_IPV6)))
> + return ERR_PTR(-EIO);
> +
> + return __udp_gso_segment(gso_skb, features, mss,
> + udp_v6_check(sizeof(struct udphdr) + mss,
> + &ip6h->saddr, &ip6h->daddr, 0));
> +}
> +
> static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> netdev_features_t features)
> {
> @@ -42,12 +56,15 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> const struct ipv6hdr *ipv6h;
> struct udphdr *uh;
>
> - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP))
> + if (!(skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP_L4)))
> goto out;
>
> if (!pskb_may_pull(skb, sizeof(struct udphdr)))
> goto out;
>
> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> + return __udp6_gso_segment(skb, features);
> +
> /* Do software UFO. Complete and fill in the UDP checksum as HW cannot
> * do checksum of UDP packets sent as multiple IP fragments.
> */
> --
> 2.17.0.441.gb46fe60e1d-goog
>
^ permalink raw reply
* Re: [PATCH RFC 2/9] veth: Add driver XDP
From: Jesper Dangaard Brouer @ 2018-04-25 20:39 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: brouer, netdev, Toshiaki Makita
In-Reply-To: <20180424143923.26519-3-toshiaki.makita1@gmail.com>
On Tue, 24 Apr 2018 23:39:16 +0900
Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
> This is basic implementation of veth driver XDP.
>
> Incoming packets are sent from the peer veth device in the form of skb,
> so this is generally doing the same thing as generic XDP.
I'm unsure that context you are calling veth_xdp_rcv_skb() from. The
XDP (RX side) depend heavily on the protection provided by NAPI context.
It looks like you are adding NAPI handler later.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: Willem de Bruijn @ 2018-04-25 20:51 UTC (permalink / raw)
To: Alexander Duyck
Cc: Netdev, David Miller, Dimitris Michailidis, Willem de Bruijn
In-Reply-To: <CAKgT0UfP7ztrtV7smQviAXZyaiPAwCSARuKnbKnc3SP_R1ogsQ@mail.gmail.com>
> It might be nice if you could break this into two patches. One for
> actually doing the GSO in software, and another enabling the stack to
> request it.
Okay.
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index d274059529eb..a4a5c0c5cba8 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -573,6 +573,8 @@ enum {
>> SKB_GSO_ESP = 1 << 15,
>>
>> SKB_GSO_UDP = 1 << 16,
>> +
>> + SKB_GSO_UDP_L4 = 1 << 17,
>> };
>>
>
> Part of me really wishes we could just rename SKB_GSO_UDP to something
> like SKB_GFO_UDP since GSO implies "segmentation" but what we really
> mean is "fragmentation" in the case of the existing UDP offload.
I have been itching to rename the UFO flag, though I then find
SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
than SKB_GFO_UDP.
But, the uapi headers will continue to have
VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
might only further complicate things.
>> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>> + netdev_features_t features,
>> + unsigned int mss, __sum16 check)
>> +{
>> + struct udphdr *uh = udp_hdr(gso_skb);
>> + struct sk_buff *segs;
>> + unsigned int hdrlen;
>> +
>> + if (gso_skb->len <= sizeof(*uh) + mss)
>> + return ERR_PTR(-EINVAL);
>> +
>> + uh->len = htons(sizeof(*uh) + mss);
>> + uh->check = check;
>> + skb_pull(gso_skb, sizeof(*uh));
>> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
>
> Normally I don't believe we modify the headers before calling
> skb_segment. Normally that happens after via a while (skb->next) {}
> type loop.
Setting the header for all but the last segment before segmentation
avoided the need for a while loop, at least before the wmem_alloc
patch.
With that patch, the argument no longer really holds, so I can convert
if that is needed for other features, like GSO_PARTIAL.
> That way for things like GSO_PARTIAL we can update after segmentation
> since there are only going to be 2 segments most likely instead of
> multiple MSS sized segments.
I don't quite follow. Which two segments?
>> +
>> + segs = skb_segment(gso_skb, features);
>> + if (unlikely(IS_ERR_OR_NULL(segs)))
>> + return segs;
>> +
>> + /* If last packet is not full, fix up its header */
>> + if (segs->prev->len != hdrlen + mss) {
>> + unsigned int mss_last = segs->prev->len - hdrlen;
>> +
>> + uh = udp_hdr(segs->prev);
>> + uh->len = htons(sizeof(*uh) + mss_last);
>> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
>> + }
>> +
>
> You could probably just assume that this last segment is always going
> to need to be updated regardless. If you are doing any sort of
> segmentation the last segment will always need the length and checksum
> updated anyway, and since you probably need to move over to the "while
> (skb->next)" style loop then you could just assume the last segment
> needs updating.
Ack. If we have to do a loop and set everything in there, I'll just write
all headers unconditionally.
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: David Miller @ 2018-04-25 20:54 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: alexander.duyck, netdev, dmichail, willemb
In-Reply-To: <CAF=yD-K2Oqf14oC-iH2sj+0cWdimdoAuY4HOYgF=BXVFbQMhEQ@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 25 Apr 2018 16:51:41 -0400
> I have been itching to rename the UFO flag, though I then find
> SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
> than SKB_GFO_UDP.
>
> But, the uapi headers will continue to have
> VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
> might only further complicate things.
I think changing the name makes things more complicated, exactly
because of all of this other legacy stuff.
^ permalink raw reply
* [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 20:57 UTC (permalink / raw)
To: Randy Dunlap
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
Michal Hocko, linux-mm, edumazet, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <1114eda5-9b1f-4db8-2090-556b4a37c532@infradead.org>
On Wed, 25 Apr 2018, Randy Dunlap wrote:
> On 04/25/2018 01:02 PM, Mikulas Patocka wrote:
> >
> >
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
> >
> > This patch introduces a fault-injection option "kvmalloc_fallback". This
> > option makes kvmalloc randomly fall back to vmalloc.
> >
> > Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
>
> Unfortunately,
OK - here I fixed the typos:
From: Mikulas Patocka <mpatocka@redhat.com>
Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
This patch introduces a fault-injection option "kvmalloc_fallback". This
option makes kvmalloc randomly fall back to vmalloc.
Unfortunately, some kernel code has bugs - it uses kvmalloc and then
uses DMA-API on the returned memory or frees it with kfree. Such bugs were
found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
code. This options helps to test for these bugs.
The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
It can be enabled in distribution debug kernels, so that kvmalloc abuse
can be tested by the users. The default can be overridden with
"kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
Documentation/fault-injection/fault-injection.txt | 7 +++++
include/linux/fault-inject.h | 9 +++---
kernel/futex.c | 2 -
lib/Kconfig.debug | 15 +++++++++++
mm/failslab.c | 2 -
mm/page_alloc.c | 2 -
mm/util.c | 30 ++++++++++++++++++++++
7 files changed, 60 insertions(+), 7 deletions(-)
Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
===================================================================
--- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
+++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
@@ -15,6 +15,12 @@ o fail_page_alloc
injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
+o kvmalloc_fallback
+
+ makes the function kvmalloc randomly fall back to vmalloc. This could be used
+ to detects bugs such as using DMA-API on the result of kvmalloc or freeing
+ the result of kvmalloc with free.
+
o fail_futex
injects futex deadlock and uaddr fault errors.
@@ -167,6 +173,7 @@ use the boot option:
failslab=
fail_page_alloc=
+ kvmalloc_fallback=
fail_make_request=
fail_futex=
mmc_core.fail_request=<interval>,<probability>,<space>,<times>
Index: linux-2.6/include/linux/fault-inject.h
===================================================================
--- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
@@ -31,17 +31,18 @@ struct fault_attr {
struct dentry *dname;
};
-#define FAULT_ATTR_INITIALIZER { \
+#define FAULT_ATTR_INITIALIZER(p) { \
+ .probability = (p), \
.interval = 1, \
- .times = ATOMIC_INIT(1), \
+ .times = ATOMIC_INIT((p) ? -1 : 1), \
+ .verbose = (p) ? 0 : 2, \
.require_end = ULONG_MAX, \
.stacktrace_depth = 32, \
.ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
- .verbose = 2, \
.dname = NULL, \
}
-#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
+#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
int setup_fault_attr(struct fault_attr *attr, char *str);
bool should_fail(struct fault_attr *attr, ssize_t size);
Index: linux-2.6/lib/Kconfig.debug
===================================================================
--- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
+++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
@@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
help
Provide fault-injection capability for alloc_pages().
+config FAIL_KVMALLOC_FALLBACK_PROBABILITY
+ int "Default kvmalloc fallback probability"
+ depends on FAULT_INJECTION
+ range 0 100
+ default "0"
+ help
+ This option will make kvmalloc randomly fall back to vmalloc.
+ Normally, kvmalloc falls back to vmalloc only rarely, if memory
+ is fragmented.
+
+ This option helps to detect hard-to-reproduce driver bugs, for
+ example using DMA API on the result of kvmalloc.
+
+ The default may be overridden with the kvmalloc_fallback parameter.
+
config FAIL_MAKE_REQUEST
bool "Fault-injection capability for disk IO"
depends on FAULT_INJECTION && BLOCK
Index: linux-2.6/mm/util.c
===================================================================
--- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
+++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
@@ -14,6 +14,7 @@
#include <linux/hugetlb.h>
#include <linux/vmalloc.h>
#include <linux/userfaultfd_k.h>
+#include <linux/fault-inject.h>
#include <asm/sections.h>
#include <linux/uaccess.h>
@@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
}
EXPORT_SYMBOL(vm_mmap);
+#ifdef CONFIG_FAULT_INJECTION
+
+static struct fault_attr kvmalloc_fallback =
+ FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
+
+static int __init setup_kvmalloc_fallback(char *str)
+{
+ return setup_fault_attr(&kvmalloc_fallback, str);
+}
+
+__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+static int __init kvmalloc_fallback_debugfs_init(void)
+{
+ fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
+ return 0;
+}
+late_initcall(kvmalloc_fallback_debugfs_init);
+#endif
+
+#endif
+
/**
* kvmalloc_node - attempt to allocate physically contiguous memory, but upon
* failure, fall back to non-contiguous (vmalloc) allocation.
@@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
*/
WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
+#ifdef CONFIG_FAULT_INJECTION
+ if (should_fail(&kvmalloc_fallback, size))
+ goto do_vmalloc;
+#endif
+
/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks and therefore
@@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
if (ret || size <= PAGE_SIZE)
return ret;
+do_vmalloc: __maybe_unused
return __vmalloc_node_flags_caller(size, node, flags,
__builtin_return_address(0));
}
Index: linux-2.6/kernel/futex.c
===================================================================
--- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
+++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
@@ -288,7 +288,7 @@ static struct {
bool ignore_private;
} fail_futex = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_private = false,
};
Index: linux-2.6/mm/failslab.c
===================================================================
--- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
@@ -9,7 +9,7 @@ static struct {
bool ignore_gfp_reclaim;
bool cache_filter;
} failslab = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.cache_filter = false,
};
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
@@ -3055,7 +3055,7 @@ static struct {
bool ignore_gfp_reclaim;
u32 min_order;
} fail_page_alloc = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.ignore_gfp_highmem = true,
.min_order = 1,
^ permalink raw reply
* Business Proposal,
From: Mrs Zeliha Faruk @ 2018-04-25 21:00 UTC (permalink / raw)
To: Recipients
Hello Dear
Greetings to you, please I have a very important business proposal for our mutual benefit, please let me know if you are interested.
Best Regards,
Miss. Zeliha ömer Faruk
Caddesi Kristal Kule Binasi
No:215
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: Alexander Duyck @ 2018-04-25 21:02 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Netdev, David Miller, Dimitris Michailidis, Willem de Bruijn
In-Reply-To: <CAF=yD-K2Oqf14oC-iH2sj+0cWdimdoAuY4HOYgF=BXVFbQMhEQ@mail.gmail.com>
On Wed, Apr 25, 2018 at 1:51 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>> It might be nice if you could break this into two patches. One for
>> actually doing the GSO in software, and another enabling the stack to
>> request it.
>
> Okay.
>
>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>> index d274059529eb..a4a5c0c5cba8 100644
>>> --- a/include/linux/skbuff.h
>>> +++ b/include/linux/skbuff.h
>>> @@ -573,6 +573,8 @@ enum {
>>> SKB_GSO_ESP = 1 << 15,
>>>
>>> SKB_GSO_UDP = 1 << 16,
>>> +
>>> + SKB_GSO_UDP_L4 = 1 << 17,
>>> };
>>>
>>
>> Part of me really wishes we could just rename SKB_GSO_UDP to something
>> like SKB_GFO_UDP since GSO implies "segmentation" but what we really
>> mean is "fragmentation" in the case of the existing UDP offload.
>
> I have been itching to rename the UFO flag, though I then find
> SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
> than SKB_GFO_UDP.
>
> But, the uapi headers will continue to have
> VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
> might only further complicate things.
Agreed. I was just venting frustration that we didn't do something
about it sooner.
>>> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>>> + netdev_features_t features,
>>> + unsigned int mss, __sum16 check)
>>> +{
>>> + struct udphdr *uh = udp_hdr(gso_skb);
>>> + struct sk_buff *segs;
>>> + unsigned int hdrlen;
>>> +
>>> + if (gso_skb->len <= sizeof(*uh) + mss)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + uh->len = htons(sizeof(*uh) + mss);
>>> + uh->check = check;
>>> + skb_pull(gso_skb, sizeof(*uh));
>>> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
>>
>> Normally I don't believe we modify the headers before calling
>> skb_segment. Normally that happens after via a while (skb->next) {}
>> type loop.
>
> Setting the header for all but the last segment before segmentation
> avoided the need for a while loop, at least before the wmem_alloc
> patch.
>
> With that patch, the argument no longer really holds, so I can convert
> if that is needed for other features, like GSO_PARTIAL.
Thanks.
>> That way for things like GSO_PARTIAL we can update after segmentation
>> since there are only going to be 2 segments most likely instead of
>> multiple MSS sized segments.
>
> I don't quite follow. Which two segments?
When we do GSO partial we end up with 2 segments. One really big one
that is a multiple of MSS and the remainder assuming the frame is odd
sized. The idea is we can just replicate all of the headers from the
outer IP header to the inner transport header in hardware so we do all
the updates based on that assumption and then we do the standard
segmentation update on the tail skb.
>>> +
>>> + segs = skb_segment(gso_skb, features);
>>> + if (unlikely(IS_ERR_OR_NULL(segs)))
>>> + return segs;
>>> +
>>> + /* If last packet is not full, fix up its header */
>>> + if (segs->prev->len != hdrlen + mss) {
>>> + unsigned int mss_last = segs->prev->len - hdrlen;
>>> +
>>> + uh = udp_hdr(segs->prev);
>>> + uh->len = htons(sizeof(*uh) + mss_last);
>>> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
>>> + }
>>> +
>>
>> You could probably just assume that this last segment is always going
>> to need to be updated regardless. If you are doing any sort of
>> segmentation the last segment will always need the length and checksum
>> updated anyway, and since you probably need to move over to the "while
>> (skb->next)" style loop then you could just assume the last segment
>> needs updating.
>
> Ack. If we have to do a loop and set everything in there, I'll just write
> all headers unconditionally.
^ permalink raw reply
* Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
From: Jakub Kicinski @ 2018-04-25 21:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, lkml, netdev, Quentin Monnet
In-Reply-To: <20180425174108.6586-4-jolsa@kernel.org>
On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> printf("tag ");
> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
3 nit picks:
Other "fields" are separated by two spaces between each other:
4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
^^ ^^ X
loaded_at Apr 25/11:20 uid 0
^^
xlated 16B not jited memlock 4096B
^^ ^^
Could you also update the example outputs in the man page:
tools/bpf/bpftool/Documentation/bpftool-prog.rst
Sorry about the bike shedding but I would also vote for:
"[not] GPL compatible"
rather than
"license GPL [NON] compatible"
for brevity..
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Randy Dunlap @ 2018-04-25 21:11 UTC (permalink / raw)
To: Mikulas Patocka
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
Michal Hocko, linux-mm, edumazet, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251656300.9428@file01.intranet.prod.int.rdu2.redhat.com>
On 04/25/2018 01:57 PM, Mikulas Patocka wrote:
>
>
> On Wed, 25 Apr 2018, Randy Dunlap wrote:
>
>> On 04/25/2018 01:02 PM, Mikulas Patocka wrote:
>>>
>>>
>>> From: Mikulas Patocka <mpatocka@redhat.com>
>>> Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
>>>
>>> This patch introduces a fault-injection option "kvmalloc_fallback". This
>>> option makes kvmalloc randomly fall back to vmalloc.
>>>
>>> Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
>>
>> Unfortunately,
>
> OK - here I fixed the typos:
>
>
> From: Mikulas Patocka <mpatocka@redhat.com>
> Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
>
> This patch introduces a fault-injection option "kvmalloc_fallback". This
> option makes kvmalloc randomly fall back to vmalloc.
>
> Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> code. This options helps to test for these bugs.
>
> The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> It can be enabled in distribution debug kernels, so that kvmalloc abuse
> can be tested by the users. The default can be overridden with
> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> Documentation/fault-injection/fault-injection.txt | 7 +++++
> include/linux/fault-inject.h | 9 +++---
> kernel/futex.c | 2 -
> lib/Kconfig.debug | 15 +++++++++++
> mm/failslab.c | 2 -
> mm/page_alloc.c | 2 -
> mm/util.c | 30 ++++++++++++++++++++++
> 7 files changed, 60 insertions(+), 7 deletions(-)
Acked-by: Randy Dunlap <rdunlap@infradead.org> # Documentation and Kconfig only
thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
From: Daniel Borkmann @ 2018-04-25 21:14 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Olsa
Cc: Alexei Starovoitov, lkml, netdev, Quentin Monnet
In-Reply-To: <20180425140346.3e0f3ba7@cakuba.netronome.com>
On 04/25/2018 11:03 PM, Jakub Kicinski wrote:
> On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
>> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
>> printf("tag ");
>> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
>> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
>> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
>
> 3 nit picks:
>
> Other "fields" are separated by two spaces between each other:
>
> 4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
> ^^ ^^ X
> loaded_at Apr 25/11:20 uid 0
> ^^
> xlated 16B not jited memlock 4096B
> ^^ ^^
>
> Could you also update the example outputs in the man page:
>
> tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> Sorry about the bike shedding but I would also vote for:
>
> "[not] GPL compatible"
>
> rather than
>
> "license GPL [NON] compatible"
>
> for brevity..
While we're at it, can we also squeeze this whole thing a bit? Feels like
huge string wasted for very little information compared to the rest of the
dump. Just append the string "gpl" at the end of the line if info->gpl_compatible
is set, otherwise just add nothing. This also allows to naturally grep
for it e.g. `bpftool p | grep gpl` if you need a quick summary.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: David Rientjes @ 2018-04-25 21:18 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Randy Dunlap, Michal Hocko, Matthew Wilcox, David Miller,
Andrew Morton, linux-mm, eric.dumazet, edumazet, netdev,
linux-kernel, mst, jasowang, virtualization, dm-devel,
Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251656300.9428@file01.intranet.prod.int.rdu2.redhat.com>
On Wed, 25 Apr 2018, Mikulas Patocka wrote:
> From: Mikulas Patocka <mpatocka@redhat.com>
> Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
>
> This patch introduces a fault-injection option "kvmalloc_fallback". This
> option makes kvmalloc randomly fall back to vmalloc.
>
> Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> code. This options helps to test for these bugs.
>
> The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> It can be enabled in distribution debug kernels, so that kvmalloc abuse
> can be tested by the users. The default can be overridden with
> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
>
Do we really need the new config option? This could just be manually
tunable via fault injection IIUC.
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> Documentation/fault-injection/fault-injection.txt | 7 +++++
> include/linux/fault-inject.h | 9 +++---
> kernel/futex.c | 2 -
> lib/Kconfig.debug | 15 +++++++++++
> mm/failslab.c | 2 -
> mm/page_alloc.c | 2 -
> mm/util.c | 30 ++++++++++++++++++++++
> 7 files changed, 60 insertions(+), 7 deletions(-)
>
> Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
> +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
> @@ -15,6 +15,12 @@ o fail_page_alloc
>
> injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
>
> +o kvmalloc_fallback
> +
> + makes the function kvmalloc randomly fall back to vmalloc. This could be used
> + to detects bugs such as using DMA-API on the result of kvmalloc or freeing
> + the result of kvmalloc with free.
> +
> o fail_futex
>
> injects futex deadlock and uaddr fault errors.
> @@ -167,6 +173,7 @@ use the boot option:
>
> failslab=
> fail_page_alloc=
> + kvmalloc_fallback=
> fail_make_request=
> fail_futex=
> mmc_core.fail_request=<interval>,<probability>,<space>,<times>
> Index: linux-2.6/include/linux/fault-inject.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
> @@ -31,17 +31,18 @@ struct fault_attr {
> struct dentry *dname;
> };
>
> -#define FAULT_ATTR_INITIALIZER { \
> +#define FAULT_ATTR_INITIALIZER(p) { \
> + .probability = (p), \
> .interval = 1, \
> - .times = ATOMIC_INIT(1), \
> + .times = ATOMIC_INIT((p) ? -1 : 1), \
> + .verbose = (p) ? 0 : 2, \
> .require_end = ULONG_MAX, \
> .stacktrace_depth = 32, \
> .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
> - .verbose = 2, \
> .dname = NULL, \
> }
>
> -#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
> +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
> int setup_fault_attr(struct fault_attr *attr, char *str);
> bool should_fail(struct fault_attr *attr, ssize_t size);
>
> Index: linux-2.6/lib/Kconfig.debug
> ===================================================================
> --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
> +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
> @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
> help
> Provide fault-injection capability for alloc_pages().
>
> +config FAIL_KVMALLOC_FALLBACK_PROBABILITY
> + int "Default kvmalloc fallback probability"
> + depends on FAULT_INJECTION
> + range 0 100
> + default "0"
> + help
> + This option will make kvmalloc randomly fall back to vmalloc.
> + Normally, kvmalloc falls back to vmalloc only rarely, if memory
> + is fragmented.
> +
> + This option helps to detect hard-to-reproduce driver bugs, for
> + example using DMA API on the result of kvmalloc.
> +
> + The default may be overridden with the kvmalloc_fallback parameter.
> +
> config FAIL_MAKE_REQUEST
> bool "Fault-injection capability for disk IO"
> depends on FAULT_INJECTION && BLOCK
> Index: linux-2.6/mm/util.c
> ===================================================================
> --- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
> +++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
> @@ -14,6 +14,7 @@
> #include <linux/hugetlb.h>
> #include <linux/vmalloc.h>
> #include <linux/userfaultfd_k.h>
> +#include <linux/fault-inject.h>
>
> #include <asm/sections.h>
> #include <linux/uaccess.h>
> @@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
> }
> EXPORT_SYMBOL(vm_mmap);
>
> +#ifdef CONFIG_FAULT_INJECTION
> +
> +static struct fault_attr kvmalloc_fallback =
> + FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
> +
> +static int __init setup_kvmalloc_fallback(char *str)
> +{
> + return setup_fault_attr(&kvmalloc_fallback, str);
> +}
> +
> +__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
> +
> +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> +static int __init kvmalloc_fallback_debugfs_init(void)
> +{
> + fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
> + return 0;
> +}
> +late_initcall(kvmalloc_fallback_debugfs_init);
> +#endif
> +
> +#endif
> +
> /**
> * kvmalloc_node - attempt to allocate physically contiguous memory, but upon
> * failure, fall back to non-contiguous (vmalloc) allocation.
> @@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
> */
> WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
>
> +#ifdef CONFIG_FAULT_INJECTION
> + if (should_fail(&kvmalloc_fallback, size))
> + goto do_vmalloc;
> +#endif
> +
> /*
> * We want to attempt a large physically contiguous block first because
> * it is less likely to fragment multiple larger blocks and therefore
> @@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
> if (ret || size <= PAGE_SIZE)
> return ret;
>
> +do_vmalloc: __maybe_unused
> return __vmalloc_node_flags_caller(size, node, flags,
> __builtin_return_address(0));
> }
> Index: linux-2.6/kernel/futex.c
> ===================================================================
> --- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
> +++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
> @@ -288,7 +288,7 @@ static struct {
>
> bool ignore_private;
> } fail_futex = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_private = false,
> };
>
> Index: linux-2.6/mm/failslab.c
> ===================================================================
> --- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
> @@ -9,7 +9,7 @@ static struct {
> bool ignore_gfp_reclaim;
> bool cache_filter;
> } failslab = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_gfp_reclaim = true,
> .cache_filter = false,
> };
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
> @@ -3055,7 +3055,7 @@ static struct {
> bool ignore_gfp_reclaim;
> u32 min_order;
> } fail_page_alloc = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_gfp_reclaim = true,
> .ignore_gfp_highmem = true,
> .min_order = 1,
>
>
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 21:22 UTC (permalink / raw)
To: David Rientjes
Cc: Randy Dunlap, Michal Hocko, Matthew Wilcox, David Miller,
Andrew Morton, linux-mm, eric.dumazet, edumazet, netdev,
linux-kernel, mst, jasowang, virtualization, dm-devel,
Vlastimil Babka
In-Reply-To: <alpine.DEB.2.21.1804251417470.166306@chino.kir.corp.google.com>
On Wed, 25 Apr 2018, David Rientjes wrote:
> On Wed, 25 Apr 2018, Mikulas Patocka wrote:
>
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
> >
> > This patch introduces a fault-injection option "kvmalloc_fallback". This
> > option makes kvmalloc randomly fall back to vmalloc.
> >
> > Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> > uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> > found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> > code. This options helps to test for these bugs.
> >
> > The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> > It can be enabled in distribution debug kernels, so that kvmalloc abuse
> > can be tested by the users. The default can be overridden with
> > "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
> >
>
> Do we really need the new config option? This could just be manually
> tunable via fault injection IIUC.
We do, because we want to enable it in RHEL and Fedora debugging kernels,
so that it will be tested by the users.
The users won't use some extra magic kernel options or debugfs files.
Mikulas
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >
> > ---
> > Documentation/fault-injection/fault-injection.txt | 7 +++++
> > include/linux/fault-inject.h | 9 +++---
> > kernel/futex.c | 2 -
> > lib/Kconfig.debug | 15 +++++++++++
> > mm/failslab.c | 2 -
> > mm/page_alloc.c | 2 -
> > mm/util.c | 30 ++++++++++++++++++++++
> > 7 files changed, 60 insertions(+), 7 deletions(-)
> >
> > Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
> > +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
> > @@ -15,6 +15,12 @@ o fail_page_alloc
> >
> > injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
> >
> > +o kvmalloc_fallback
> > +
> > + makes the function kvmalloc randomly fall back to vmalloc. This could be used
> > + to detects bugs such as using DMA-API on the result of kvmalloc or freeing
> > + the result of kvmalloc with free.
> > +
> > o fail_futex
> >
> > injects futex deadlock and uaddr fault errors.
> > @@ -167,6 +173,7 @@ use the boot option:
> >
> > failslab=
> > fail_page_alloc=
> > + kvmalloc_fallback=
> > fail_make_request=
> > fail_futex=
> > mmc_core.fail_request=<interval>,<probability>,<space>,<times>
> > Index: linux-2.6/include/linux/fault-inject.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
> > @@ -31,17 +31,18 @@ struct fault_attr {
> > struct dentry *dname;
> > };
> >
> > -#define FAULT_ATTR_INITIALIZER { \
> > +#define FAULT_ATTR_INITIALIZER(p) { \
> > + .probability = (p), \
> > .interval = 1, \
> > - .times = ATOMIC_INIT(1), \
> > + .times = ATOMIC_INIT((p) ? -1 : 1), \
> > + .verbose = (p) ? 0 : 2, \
> > .require_end = ULONG_MAX, \
> > .stacktrace_depth = 32, \
> > .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
> > - .verbose = 2, \
> > .dname = NULL, \
> > }
> >
> > -#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
> > +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
> > int setup_fault_attr(struct fault_attr *attr, char *str);
> > bool should_fail(struct fault_attr *attr, ssize_t size);
> >
> > Index: linux-2.6/lib/Kconfig.debug
> > ===================================================================
> > --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
> > +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
> > @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
> > help
> > Provide fault-injection capability for alloc_pages().
> >
> > +config FAIL_KVMALLOC_FALLBACK_PROBABILITY
> > + int "Default kvmalloc fallback probability"
> > + depends on FAULT_INJECTION
> > + range 0 100
> > + default "0"
> > + help
> > + This option will make kvmalloc randomly fall back to vmalloc.
> > + Normally, kvmalloc falls back to vmalloc only rarely, if memory
> > + is fragmented.
> > +
> > + This option helps to detect hard-to-reproduce driver bugs, for
> > + example using DMA API on the result of kvmalloc.
> > +
> > + The default may be overridden with the kvmalloc_fallback parameter.
> > +
> > config FAIL_MAKE_REQUEST
> > bool "Fault-injection capability for disk IO"
> > depends on FAULT_INJECTION && BLOCK
> > Index: linux-2.6/mm/util.c
> > ===================================================================
> > --- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
> > +++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
> > @@ -14,6 +14,7 @@
> > #include <linux/hugetlb.h>
> > #include <linux/vmalloc.h>
> > #include <linux/userfaultfd_k.h>
> > +#include <linux/fault-inject.h>
> >
> > #include <asm/sections.h>
> > #include <linux/uaccess.h>
> > @@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
> > }
> > EXPORT_SYMBOL(vm_mmap);
> >
> > +#ifdef CONFIG_FAULT_INJECTION
> > +
> > +static struct fault_attr kvmalloc_fallback =
> > + FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
> > +
> > +static int __init setup_kvmalloc_fallback(char *str)
> > +{
> > + return setup_fault_attr(&kvmalloc_fallback, str);
> > +}
> > +
> > +__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
> > +
> > +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> > +static int __init kvmalloc_fallback_debugfs_init(void)
> > +{
> > + fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
> > + return 0;
> > +}
> > +late_initcall(kvmalloc_fallback_debugfs_init);
> > +#endif
> > +
> > +#endif
> > +
> > /**
> > * kvmalloc_node - attempt to allocate physically contiguous memory, but upon
> > * failure, fall back to non-contiguous (vmalloc) allocation.
> > @@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
> > */
> > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
> >
> > +#ifdef CONFIG_FAULT_INJECTION
> > + if (should_fail(&kvmalloc_fallback, size))
> > + goto do_vmalloc;
> > +#endif
> > +
> > /*
> > * We want to attempt a large physically contiguous block first because
> > * it is less likely to fragment multiple larger blocks and therefore
> > @@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
> > if (ret || size <= PAGE_SIZE)
> > return ret;
> >
> > +do_vmalloc: __maybe_unused
> > return __vmalloc_node_flags_caller(size, node, flags,
> > __builtin_return_address(0));
> > }
> > Index: linux-2.6/kernel/futex.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
> > +++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
> > @@ -288,7 +288,7 @@ static struct {
> >
> > bool ignore_private;
> > } fail_futex = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_private = false,
> > };
> >
> > Index: linux-2.6/mm/failslab.c
> > ===================================================================
> > --- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
> > @@ -9,7 +9,7 @@ static struct {
> > bool ignore_gfp_reclaim;
> > bool cache_filter;
> > } failslab = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_gfp_reclaim = true,
> > .cache_filter = false,
> > };
> > Index: linux-2.6/mm/page_alloc.c
> > ===================================================================
> > --- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
> > @@ -3055,7 +3055,7 @@ static struct {
> > bool ignore_gfp_reclaim;
> > u32 min_order;
> > } fail_page_alloc = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_gfp_reclaim = true,
> > .ignore_gfp_highmem = true,
> > .min_order = 1,
> >
> >
>
^ permalink raw reply
* [bpf PATCH v2] bpf: fix for lex/yacc build error with gcc-5
From: John Fastabend @ 2018-04-25 21:22 UTC (permalink / raw)
To: ast, daniel, jbenc; +Cc: netdev
Fix build error found with Ubuntu shipped gcc-5
~/git/bpf/tools/bpf$ make all
Auto-detecting system features:
... libbfd: [ OFF ]
... disassembler-four-args: [ OFF ]
CC bpf_jit_disasm.o
LINK bpf_jit_disasm
CC bpf_dbg.o
/home/john/git/bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
/home/john/git/bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
} else if (matches(subcmd, "pcap") == 0) {
^
LINK bpf_dbg
CC bpf_asm.o
make: *** No rule to make target `bpf_exp.yacc.o', needed by `bpf_asm'. Stop.
Fixes: 5a8997f20715 ("tools: bpf: respect output directory during build")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/bpf/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 1ea5459..53b60ad 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -76,6 +76,8 @@ $(OUTPUT)bpf_asm: $(OUTPUT)bpf_asm.o $(OUTPUT)bpf_exp.yacc.o $(OUTPUT)bpf_exp.le
$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^
$(OUTPUT)bpf_exp.lex.c: $(OUTPUT)bpf_exp.yacc.c
+$(OUTPUT)bpf_exp.yacc.o: $(OUTPUT)bpf_exp.yacc.c
+$(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
clean: bpftool_clean
$(call QUIET_CLEAN, bpf-progs)
^ permalink raw reply related
* Re: simplify procfs code for seq_file instances
From: Alexey Dobriyan @ 2018-04-25 21:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-rtc, Alessandro Zummo, Alexandre Belloni, devel, linux-scsi,
Corey Minyard, linux-ide, Greg Kroah-Hartman, jfs-discussion,
linux-kernel, linux-acpi, netdev, netfilter-devel, Alexander Viro,
Jiri Slaby, Andrew Morton, linux-ext4, linux-afs,
megaraidlinux.pdl, drbd-dev
In-Reply-To: <20180424160652.GA28483@lst.de>
On Tue, Apr 24, 2018 at 06:06:53PM +0200, Christoph Hellwig wrote:
> On Tue, Apr 24, 2018 at 08:19:16AM -0700, Andrew Morton wrote:
> > > > I want to ask if it is time to start using poorman function overloading
> > > > with _b_c_e(). There are millions of allocation functions for example,
> > > > all slightly difference, and people will add more. Seeing /proc interfaces
> > > > doubled like this is painful.
> > >
> > > Function overloading is totally unacceptable.
> > >
> > > And I very much disagree with a tradeoff that keeps 5000 lines of
> > > code vs a few new helpers.
> >
> > OK, the curiosity and suspense are killing me. What the heck is
> > "function overloading with _b_c_e()"?
>
> The way I understood Alexey was to use have a proc_create macro
> that can take different ops types. Although the short cut for
> __builtin_types_compatible_p would be _b_t_c or similar, so maybe
> I misunderstood him.
That's correct.
I also think that several dozens kmalloc signatures are a problem.
And there will be more with pmalloc* stuff and more 2D/3D array
checked allocations and who knows what.
And I want to add typed kmalloc!
^ permalink raw reply
* [PATCH v2] selftests: net: add in_netns.sh TEST_GEN_PROGS_EXTENDED
From: Anders Roxell @ 2018-04-25 21:32 UTC (permalink / raw)
To: davem, shuah; +Cc: netdev, linux-kselftest, linux-kernel, Anders Roxell
In-Reply-To: <20180425.132513.1786379382285267245.davem@davemloft.net>
Script in_netns.sh is a utility function and not its own test so it
shouldn't be part of the TEST_PROGS. The in_netns.sh get used by
run_afpackettests.
To install in_netns.sh without being added to the main run_kselftest.sh
script use the TEST_GEN_PROGS_EXTENDED variable.
Fixes: 5ff9c1a3dd92 ("selftests: net: add in_netns.sh to TEST_PROGS")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
respin against the 'net' tree.
tools/testing/selftests/net/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 8f1e13d2e547..daf5effec3f0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,7 +5,8 @@ CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
CFLAGS += -I../../../../usr/include/
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
-TEST_PROGS += fib_tests.sh fib-onlink-tests.sh in_netns.sh pmtu.sh
+TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh
+TEST_GEN_PROGS_EXTENDED := in_netns.sh
TEST_GEN_FILES = socket
TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: Siwei Liu @ 2018-04-25 21:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Sridhar Samudrala, virtualization, Netdev, David Miller
In-Reply-To: <20180423230037-mutt-send-email-mst@kernel.org>
On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >>
>> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> > > > >
>> >> > > > >I will NAK patches to change to common code for netvsc especially the
>> >> > > > >three device model. MS worked hard with distro vendors to support transparent
>> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> > > > >
>> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> > > >
>> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
>> >> > >
>> >> > > The network device model is a userspace API, and DPDK is a userspace application.
>> >> >
>> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> >> > AFAIK it's normally banging device registers directly.
>> >> >
>> >> > > You can't go breaking userspace even if you don't like the application.
>> >> >
>> >> > Could you please explain how is the proposed patchset breaking
>> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
>> >> > API at all.
>> >> >
>> >>
>> >> The DPDK has a device driver vdev_netvsc which scans the Linux network devices
>> >> to look for Linux netvsc device and the paired VF device and setup the
>> >> DPDK environment. This setup creates a DPDK failsafe (bondingish) instance
>> >> and sets up TAP support over the Linux netvsc device as well as the Mellanox
>> >> VF device.
>> >>
>> >> So it depends on existing 2 device model. You can't go to a 3 device model
>> >> or start hiding devices from userspace.
>> >
>> > Okay so how does the existing patch break that? IIUC does not go to
>> > a 3 device model since netvsc calls failover_register directly.
>> >
>> >> Also, I am working on associating netvsc and VF device based on serial number
>> >> rather than MAC address. The serial number is how Windows works now, and it makes
>> >> sense for Linux and Windows to use the same mechanism if possible.
>> >
>> > Maybe we should support same for virtio ...
>> > Which serial do you mean? From vpd?
>> >
>> > I guess you will want to keep supporting MAC for old hypervisors?
>> >
>> > It all seems like a reasonable thing to support in the generic core.
>>
>> That's the reason why I chose explicit identifier rather than rely on
>> MAC address to bind/pair a device. MAC address can change. Even if it
>> can't, malicious guest user can fake MAC address to skip binding.
>>
>> -Siwei
>
> Address should be sampled at device creation to prevent this
> kind of hack. Not that it buys the malicious user much:
> if you can poke at MAC addresses you probably already can
> break networking.
I don't understand why poking at MAC address may potentially break
networking. Unlike VF, passthrough PCI endpoint device has its freedom
to change the MAC address. Even on a VF setup it's not neccessarily
always safe to assume the VF's MAC address cannot or shouldn't be
changed. That depends on the specific need whether the host admin
wants to restrict guest from changing the MAC address, although in
most cases it's true.
I understand we can use the perm_addr to distinguish. But as said,
this will pose limitation of flexible configuration where one can
assign VFs with identical MAC address at all while each VF belongs to
different PF and/or different subnet for e.g. load balancing. And
furthermore, the QEMU device model never uses MAC address to be
interpreted as an identifier, which requires to be unique per VM
instance. Why we're introducing this inconsistency?
-Siwei
>
>
>
>
>>
>> >
>> > --
>> > MST
^ permalink raw reply
* Re: [PATCH][net-next] net: ip tos cgroup
From: kbuild test robot @ 2018-04-25 21:39 UTC (permalink / raw)
To: Li RongQing; +Cc: kbuild-all, netdev
In-Reply-To: <1523936161-11676-1-git-send-email-lirongqing@baidu.com>
[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]
Hi Li,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Li-RongQing/net-ip-tos-cgroup/20180417-204807
config: arm-dove_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
In file included from net/ipv4/af_inet.c:123:0:
>> include/net/tos_cgroup.h:10:29: error: field 'css' has incomplete type
struct cgroup_subsys_state css;
^~~
vim +/css +10 include/net/tos_cgroup.h
8
9 struct tos_cgroup_state {
> 10 struct cgroup_subsys_state css;
11 u32 tos;
12 };
13
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17596 bytes --]
^ permalink raw reply
* Re: [bpf PATCH v2] bpf: fix for lex/yacc build error with gcc-5
From: Daniel Borkmann @ 2018-04-25 21:42 UTC (permalink / raw)
To: John Fastabend, ast, jbenc; +Cc: netdev
In-Reply-To: <20180425212245.25999.21181.stgit@john-Precision-Tower-5810>
On 04/25/2018 11:22 PM, John Fastabend wrote:
> Fix build error found with Ubuntu shipped gcc-5
>
> ~/git/bpf/tools/bpf$ make all
>
> Auto-detecting system features:
> ... libbfd: [ OFF ]
> ... disassembler-four-args: [ OFF ]
>
> CC bpf_jit_disasm.o
> LINK bpf_jit_disasm
> CC bpf_dbg.o
> /home/john/git/bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
> /home/john/git/bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> } else if (matches(subcmd, "pcap") == 0) {
> ^
> LINK bpf_dbg
> CC bpf_asm.o
> make: *** No rule to make target `bpf_exp.yacc.o', needed by `bpf_asm'. Stop.
>
> Fixes: 5a8997f20715 ("tools: bpf: respect output directory during build")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Applied to bpf, thanks John! Will see to get that annoying bpf_dbg warn out
of the way if no one beats me to it. ;)
^ permalink raw reply
* [PATCH v2 net-next 0/2] tcp: mmap: rework zerocopy receive
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet
syzbot reported a lockdep issue caused by tcp mmap() support.
I implemented Andy Lutomirski nice suggestions to resolve the
issue and increase scalability as well.
First patch is adding a new setsockopt() operation and changes mmap()
behavior.
Second patch changes tcp_mmap reference program.
v2:
Added a missing page align of zc->length in tcp_zerocopy_receive()
Properly clear zc->recv_skip_hint in case user request was completed.
Eric Dumazet (2):
tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
selftests: net: tcp_mmap must use TCP_ZEROCOPY_RECEIVE
include/uapi/linux/tcp.h | 8 ++
net/ipv4/tcp.c | 189 +++++++++++++------------
tools/testing/selftests/net/tcp_mmap.c | 63 +++++----
3 files changed, 142 insertions(+), 118 deletions(-)
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply
* [PATCH v2 net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet, Soheil Hassas Yeganeh
In-Reply-To: <20180425214307.159264-1-edumazet@google.com>
When adding tcp mmap() implementation, I forgot that socket lock
had to be taken before current->mm->mmap_sem. syzbot eventually caught
the bug.
Since we can not lock the socket in tcp mmap() handler we have to
split the operation in two phases.
1) mmap() on a tcp socket simply reserves VMA space, and nothing else.
This operation does not involve any TCP locking.
2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements
the transfert of pages from skbs to one VMA.
This operation only uses down_read(¤t->mm->mmap_sem) after
holding TCP lock, thus solving the lockdep issue.
This new implementation was suggested by Andy Lutomirski with great details.
Benefits are :
- Better scalability, in case multiple threads reuse VMAS
(without mmap()/munmap() calls) since mmap_sem wont be write locked.
- Better error recovery.
The previous mmap() model had to provide the expected size of the
mapping. If for some reason one part could not be mapped (partial MSS),
the whole operation had to be aborted.
With the tcp_zerocopy_receive struct, kernel can report how
many bytes were successfuly mapped, and how many bytes should
be read to skip the problematic sequence.
- No more memory allocation to hold an array of page pointers.
16 MB mappings needed 32 KB for this array, potentially using vmalloc() :/
- skbs are freed while mmap_sem has been released
Following patch makes the change in tcp_mmap tool to demonstrate
one possible use of mmap() and setsockopt(... TCP_ZEROCOPY_RECEIVE ...)
Note that memcg might require additional changes.
Fixes: 93ab6cc69162 ("tcp: implement mmap() for zero copy receive")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Cc: linux-mm@kvack.org
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
include/uapi/linux/tcp.h | 8 ++
net/ipv4/tcp.c | 189 ++++++++++++++++++++-------------------
2 files changed, 106 insertions(+), 91 deletions(-)
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 379b08700a542d49bbce9b4b49b17879d00b69bb..e9e8373b34b9ddc735329341b91f455bf5c0b17c 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -122,6 +122,7 @@ enum {
#define TCP_MD5SIG_EXT 32 /* TCP MD5 Signature with extensions */
#define TCP_FASTOPEN_KEY 33 /* Set the key for Fast Open (cookie) */
#define TCP_FASTOPEN_NO_COOKIE 34 /* Enable TFO without a TFO cookie */
+#define TCP_ZEROCOPY_RECEIVE 35
struct tcp_repair_opt {
__u32 opt_code;
@@ -276,4 +277,11 @@ struct tcp_diag_md5sig {
__u8 tcpm_key[TCP_MD5SIG_MAXKEYLEN];
};
+/* setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) */
+
+struct tcp_zerocopy_receive {
+ __u64 address; /* in: address of mapping */
+ __u32 length; /* in/out: number of bytes to map/mapped */
+ __u32 recv_skip_hint; /* out: amount of bytes to skip */
+};
#endif /* _UAPI_LINUX_TCP_H */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dfd090ea54ad47112fc23c61180b5bf8edd2c736..9b9cbb837ff8d6cdd85515429f699e113c55b37b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1726,118 +1726,111 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
}
EXPORT_SYMBOL(tcp_set_rcvlowat);
-/* When user wants to mmap X pages, we first need to perform the mapping
- * before freeing any skbs in receive queue, otherwise user would be unable
- * to fallback to standard recvmsg(). This happens if some data in the
- * requested block is not exactly fitting in a page.
- *
- * We only support order-0 pages for the moment.
- * mmap() on TCP is very strict, there is no point
- * trying to accommodate with pathological layouts.
- */
+static const struct vm_operations_struct tcp_vm_ops = {
+};
+
int tcp_mmap(struct file *file, struct socket *sock,
struct vm_area_struct *vma)
{
- unsigned long size = vma->vm_end - vma->vm_start;
- unsigned int nr_pages = size >> PAGE_SHIFT;
- struct page **pages_array = NULL;
- u32 seq, len, offset, nr = 0;
- struct sock *sk = sock->sk;
- const skb_frag_t *frags;
+ if (vma->vm_flags & (VM_WRITE | VM_EXEC))
+ return -EPERM;
+ vma->vm_flags &= ~(VM_MAYWRITE | VM_MAYEXEC);
+
+ /* Instruct vm_insert_page() to not down_read(mmap_sem) */
+ vma->vm_flags |= VM_MIXEDMAP;
+
+ vma->vm_ops = &tcp_vm_ops;
+ return 0;
+}
+EXPORT_SYMBOL(tcp_mmap);
+
+static int tcp_zerocopy_receive(struct sock *sk,
+ struct tcp_zerocopy_receive *zc)
+{
+ unsigned long address = (unsigned long)zc->address;
+ const skb_frag_t *frags = NULL;
+ u32 length = 0, seq, offset;
+ struct vm_area_struct *vma;
+ struct sk_buff *skb = NULL;
struct tcp_sock *tp;
- struct sk_buff *skb;
int ret;
- if (vma->vm_pgoff || !nr_pages)
+ if (address & (PAGE_SIZE - 1) || address != zc->address)
return -EINVAL;
- if (vma->vm_flags & VM_WRITE)
- return -EPERM;
- /* TODO: Maybe the following is not needed if pages are COW */
- vma->vm_flags &= ~VM_MAYWRITE;
-
- lock_sock(sk);
-
- ret = -ENOTCONN;
if (sk->sk_state == TCP_LISTEN)
- goto out;
+ return -ENOTCONN;
sock_rps_record_flow(sk);
- if (tcp_inq(sk) < size) {
- ret = sock_flag(sk, SOCK_DONE) ? -EIO : -EAGAIN;
+ down_read(¤t->mm->mmap_sem);
+
+ ret = -EINVAL;
+ vma = find_vma(current->mm, address);
+ if (!vma || vma->vm_start > address || vma->vm_ops != &tcp_vm_ops)
goto out;
- }
+ zc->length = min_t(unsigned long, zc->length, vma->vm_end - address);
+
tp = tcp_sk(sk);
seq = tp->copied_seq;
- /* Abort if urgent data is in the area */
- if (unlikely(tp->urg_data)) {
- u32 urg_offset = tp->urg_seq - seq;
+ zc->length = min_t(u32, zc->length, tcp_inq(sk));
+ zc->length &= ~(PAGE_SIZE - 1);
- ret = -EINVAL;
- if (urg_offset < size)
- goto out;
- }
- ret = -ENOMEM;
- pages_array = kvmalloc_array(nr_pages, sizeof(struct page *),
- GFP_KERNEL);
- if (!pages_array)
- goto out;
- skb = tcp_recv_skb(sk, seq, &offset);
- ret = -EINVAL;
-skb_start:
- /* We do not support anything not in page frags */
- offset -= skb_headlen(skb);
- if ((int)offset < 0)
- goto out;
- if (skb_has_frag_list(skb))
- goto out;
- len = skb->data_len - offset;
- frags = skb_shinfo(skb)->frags;
- while (offset) {
- if (frags->size > offset)
- goto out;
- offset -= frags->size;
- frags++;
- }
- while (nr < nr_pages) {
- if (len) {
- if (len < PAGE_SIZE)
- goto out;
- if (frags->size != PAGE_SIZE || frags->page_offset)
- goto out;
- pages_array[nr++] = skb_frag_page(frags);
- frags++;
- len -= PAGE_SIZE;
- seq += PAGE_SIZE;
- continue;
- }
- skb = skb->next;
- offset = seq - TCP_SKB_CB(skb)->seq;
- goto skb_start;
- }
- /* OK, we have a full set of pages ready to be inserted into vma */
- for (nr = 0; nr < nr_pages; nr++) {
- ret = vm_insert_page(vma, vma->vm_start + (nr << PAGE_SHIFT),
- pages_array[nr]);
- if (ret)
- goto out;
- }
- /* operation is complete, we can 'consume' all skbs */
- tp->copied_seq = seq;
- tcp_rcv_space_adjust(sk);
-
- /* Clean up data we have read: This will do ACK frames. */
- tcp_recv_skb(sk, seq, &offset);
- tcp_cleanup_rbuf(sk, size);
+ zap_page_range(vma, address, zc->length);
+ zc->recv_skip_hint = 0;
ret = 0;
+ while (length + PAGE_SIZE <= zc->length) {
+ if (zc->recv_skip_hint < PAGE_SIZE) {
+ if (skb) {
+ skb = skb->next;
+ offset = seq - TCP_SKB_CB(skb)->seq;
+ } else {
+ skb = tcp_recv_skb(sk, seq, &offset);
+ }
+
+ zc->recv_skip_hint = skb->len - offset;
+ offset -= skb_headlen(skb);
+ if ((int)offset < 0 || skb_has_frag_list(skb))
+ break;
+ frags = skb_shinfo(skb)->frags;
+ while (offset) {
+ if (frags->size > offset)
+ goto out;
+ offset -= frags->size;
+ frags++;
+ }
+ }
+ if (frags->size != PAGE_SIZE || frags->page_offset)
+ break;
+ ret = vm_insert_page(vma, address + length,
+ skb_frag_page(frags));
+ if (ret)
+ break;
+ length += PAGE_SIZE;
+ seq += PAGE_SIZE;
+ zc->recv_skip_hint -= PAGE_SIZE;
+ frags++;
+ }
out:
- release_sock(sk);
- kvfree(pages_array);
+ up_read(¤t->mm->mmap_sem);
+ if (length) {
+ tp->copied_seq = seq;
+ tcp_rcv_space_adjust(sk);
+
+ /* Clean up data we have read: This will do ACK frames. */
+ tcp_recv_skb(sk, seq, &offset);
+ tcp_cleanup_rbuf(sk, length);
+ ret = 0;
+ if (length == zc->length)
+ zc->recv_skip_hint = 0;
+ } else {
+ if (!zc->recv_skip_hint && sock_flag(sk, SOCK_DONE))
+ ret = -EIO;
+ }
+ zc->length = length;
return ret;
}
-EXPORT_SYMBOL(tcp_mmap);
static void tcp_update_recv_tstamps(struct sk_buff *skb,
struct scm_timestamping *tss)
@@ -2738,6 +2731,20 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
return tcp_fastopen_reset_cipher(net, sk, key, sizeof(key));
}
+ case TCP_ZEROCOPY_RECEIVE: {
+ struct tcp_zerocopy_receive zc;
+
+ if (optlen != sizeof(zc))
+ return -EINVAL;
+ if (copy_from_user(&zc, optval, optlen))
+ return -EFAULT;
+ lock_sock(sk);
+ err = tcp_zerocopy_receive(sk, &zc);
+ release_sock(sk);
+ if (!err && copy_to_user(optval, &zc, optlen))
+ err = -EFAULT;
+ return err;
+ }
default:
/* fallthru */
break;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 2/2] selftests: net: tcp_mmap must use TCP_ZEROCOPY_RECEIVE
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet, Soheil Hassas Yeganeh
In-Reply-To: <20180425214307.159264-1-edumazet@google.com>
After prior kernel change, mmap() on TCP socket only reserves VMA.
We have to use setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...)
to perform the transfert of pages from skbs in TCP receive queue into such VMA.
struct tcp_zerocopy_receive {
__u64 address; /* in: address of mapping */
__u32 length; /* in/out: number of bytes to map/mapped */
__u32 recv_skip_hint; /* out: amount of bytes to skip */
};
After a successful setsockopt(...TCP_ZEROCOPY_RECEIVE...), @length contains
number of bytes that were mapped, and @recv_skip_hint contains number of bytes
that should be read using conventional read()/recv()/recvmsg() system calls,
to skip a sequence of bytes that can not be mapped, because not properly page
aligned.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
tools/testing/selftests/net/tcp_mmap.c | 63 +++++++++++++++-----------
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_mmap.c b/tools/testing/selftests/net/tcp_mmap.c
index dea342fe6f4e88b5709d2ac37b2fc9a2a320bf44..5b381cdbdd6319556ba4e3dad530fae8f13f5a9b 100644
--- a/tools/testing/selftests/net/tcp_mmap.c
+++ b/tools/testing/selftests/net/tcp_mmap.c
@@ -76,9 +76,10 @@
#include <time.h>
#include <sys/time.h>
#include <netinet/in.h>
-#include <netinet/tcp.h>
#include <arpa/inet.h>
#include <poll.h>
+#include <linux/tcp.h>
+#include <assert.h>
#ifndef MSG_ZEROCOPY
#define MSG_ZEROCOPY 0x4000000
@@ -134,11 +135,12 @@ void hash_zone(void *zone, unsigned int length)
void *child_thread(void *arg)
{
unsigned long total_mmap = 0, total = 0;
+ struct tcp_zerocopy_receive zc;
unsigned long delta_usec;
int flags = MAP_SHARED;
struct timeval t0, t1;
char *buffer = NULL;
- void *oaddr = NULL;
+ void *addr = NULL;
double throughput;
struct rusage ru;
int lu, fd;
@@ -153,41 +155,45 @@ void *child_thread(void *arg)
perror("malloc");
goto error;
}
+ if (zflg) {
+ addr = mmap(NULL, chunk_size, PROT_READ, flags, fd, 0);
+ if (addr == (void *)-1)
+ zflg = 0;
+ }
while (1) {
struct pollfd pfd = { .fd = fd, .events = POLLIN, };
int sub;
poll(&pfd, 1, 10000);
if (zflg) {
- void *naddr;
+ int res;
- naddr = mmap(oaddr, chunk_size, PROT_READ, flags, fd, 0);
- if (naddr == (void *)-1) {
- if (errno == EAGAIN) {
- /* That is if SO_RCVLOWAT is buggy */
- usleep(1000);
- continue;
- }
- if (errno == EINVAL) {
- flags = MAP_SHARED;
- oaddr = NULL;
- goto fallback;
- }
- if (errno != EIO)
- perror("mmap()");
+ zc.address = (__u64)addr;
+ zc.length = chunk_size;
+ zc.recv_skip_hint = 0;
+ res = setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+ &zc, sizeof(zc));
+ if (res == -1)
break;
+
+ if (zc.length) {
+ assert(zc.length <= chunk_size);
+ total_mmap += zc.length;
+ if (xflg)
+ hash_zone(addr, zc.length);
+ total += zc.length;
}
- total_mmap += chunk_size;
- if (xflg)
- hash_zone(naddr, chunk_size);
- total += chunk_size;
- if (!keepflag) {
- flags |= MAP_FIXED;
- oaddr = naddr;
+ if (zc.recv_skip_hint) {
+ assert(zc.recv_skip_hint <= chunk_size);
+ lu = read(fd, buffer, zc.recv_skip_hint);
+ if (lu > 0) {
+ if (xflg)
+ hash_zone(buffer, lu);
+ total += lu;
+ }
}
continue;
}
-fallback:
sub = 0;
while (sub < chunk_size) {
lu = read(fd, buffer + sub, chunk_size - sub);
@@ -228,6 +234,8 @@ void *child_thread(void *arg)
error:
free(buffer);
close(fd);
+ if (zflg)
+ munmap(addr, chunk_size);
pthread_exit(0);
}
@@ -371,7 +379,8 @@ int main(int argc, char *argv[])
setup_sockaddr(cfg_family, host, &listenaddr);
if (mss &&
- setsockopt(fdlisten, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
+ setsockopt(fdlisten, IPPROTO_TCP, TCP_MAXSEG,
+ &mss, sizeof(mss)) == -1) {
perror("setsockopt TCP_MAXSEG");
exit(1);
}
@@ -402,7 +411,7 @@ int main(int argc, char *argv[])
setup_sockaddr(cfg_family, host, &addr);
if (mss &&
- setsockopt(fd, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
+ setsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
perror("setsockopt TCP_MAXSEG");
exit(1);
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* Re: [PATCH net-next v2 2/2] openvswitch: Support conntrack zone limit
From: Yi-Hung Wei @ 2018-04-25 21:51 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAOrHB_DHAjUp8Mf1_d-uKiiJPqUmwOvQDCTveFuvEhj9oJ-DhQ@mail.gmail.com>
>> +#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
>> +#define OVS_CT_LIMIT_UNLIMITED 0
>> +#define OVS_CT_LIMIT_DEFAULT OVS_CT_LIMIT_UNLIMITED
>> +#define CT_LIMIT_HASH_BUCKETS 512
>> +
> Can you use static key when the limit is not set.
> This would avoid overhead in datapath when these limits are not used.
>
Thanks Parvin for the review. I'm not sure about the 'static key'
part, are you suggesting that say if we can have a flag to check if no
one has ever set the ct_limit? If the ct_limit feature is not used
so far, then instead of look up the hash table for the zone limit, we
just return the default unlimited value. So that we can avoid the
overhead of checking ct_limit.
>> +struct ovs_ct_limit {
>> + /* Elements in ovs_ct_limit_info->limits hash table */
>> + struct hlist_node hlist_node;
>> + struct rcu_head rcu;
>> + u16 zone;
>> + u32 limit;
>> +};
>> +
> ...
>> /* Lookup connection and confirm if unconfirmed. */
>> static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>> const struct ovs_conntrack_info *info,
>> @@ -1054,6 +1176,13 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>> if (!ct)
>> return 0;
>>
>> +#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
>> + err = ovs_ct_check_limit(net, info,
>> + &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
>> + if (err)
>> + return err;
>> +#endif
>> +
>
> This could be checked during flow install time, so that only permitted
> flows would have 'ct commit' action, we can avoid per packet cost
> checking the limit.
It seems to me that it would be hard to check the # of connections of
a flow in the flow installation stage. For example, if we have a
datapath flow that performs “ct commit” action on all UDP traffic from
in_port 1, then it could be various combinations of 5-tuple that form
various # of connections. Therefore, it would be hard to do the
admission control there.
> returning error code form ovs_ct_commit() is lost in datapath and it
> would be hard to debug packet lost in case of the limit is reached. So
> another advantage of checking the limit in flow install be better
> traceability. datapath would return error to usespace and it can log
> the error code.
Thanks for pointing out the error code from ovs_ct_commit() will be
lost in the datapath. In this case, shall we consider to report the
packet drop by some rate_limit logging such as net_warn_ratelimited()
or net_info_ratelimited()?
Thanks,
-Yi-Hung
^ permalink raw reply
* Loan Offer
From: Coleman Krosberg @ 2018-04-25 21:53 UTC (permalink / raw)
Thank you for your time,
We are looking for clients in your area of residents with good business or project that requires financing
to execute, or who wants a loan to do business, we do give loans with low interest.
Please get back to me if you are interested in this or you know anybody who has good business ideas but
lack the necessary capital to fund his projects or business so we can establish working relationship.
Yours Sincerely,
Coleman Krosberg MBA, CFA
Ak Bank Financial Rep
Investment analyst.
^ permalink raw reply
* pull-request: bpf 2018-04-25
From: Daniel Borkmann @ 2018-04-25 22:04 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net* tree.
The main changes are:
1) Fix to clear the percpu metadata_dst that could otherwise carry
stale ip_tunnel_info, from William.
2) Fix that reduces the number of passes in x64 JIT with regards to
dead code sanitation to avoid risk of prog rejection, from Gianluca.
3) Several fixes of sockmap programs, besides others, fixing a double
page_put() in error path, missing refcount hold for pinned sockmap,
adding required -target bpf for clang in sample Makefile, from John.
4) Fix to disable preemption in __BPF_PROG_RUN_ARRAY() paths, from Roman.
5) Fix tools/bpf/ Makefile with regards to a lex/yacc build error
seen on older gcc-5, from John.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Would be great if you have a chance to merge net into net-next after
that since sockmap fixes are needed in bpf-next later on to avoid
ugly merge conflicts.
Thanks a lot!
----------------------------------------------------------------
The following changes since commit 77621f024d6be732e43366a42203486b6ec89acd:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf (2018-04-23 16:22:24 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to 9c299a32ede98dc9faafb267034ed830a15304db:
bpf: fix for lex/yacc build error with gcc-5 (2018-04-25 23:27:46 +0200)
----------------------------------------------------------------
Daniel Borkmann (1):
Merge branch 'bpf-sockmap-fixes'
Gianluca Borello (1):
bpf, x64: fix JIT emission for dead code
John Fastabend (6):
bpf: Document sockmap '-target bpf' requirement for PROG_TYPE_SK_MSG
bpf: sockmap sample use clang flag, -target bpf
bpf: sockmap, map_release does not hold refcnt for pinned maps
bpf: sockmap, sk_wait_event needed to handle blocking cases
bpf: sockmap, fix double page_put on ENOMEM error in redirect path
bpf: fix for lex/yacc build error with gcc-5
Roman Gushchin (1):
bpf: disable and restore preemption in __BPF_PROG_RUN_ARRAY
William Tu (1):
bpf: clear the ip_tunnel_info.
Documentation/bpf/bpf_devel_QA.txt | 10 +++++++-
arch/x86/net/bpf_jit_comp.c | 12 ++++++++-
include/linux/bpf.h | 4 ++-
kernel/bpf/arraymap.c | 3 ++-
kernel/bpf/sockmap.c | 51 +++++++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 4 +--
net/core/filter.c | 1 +
samples/sockmap/Makefile | 7 ++++--
tools/bpf/Makefile | 2 ++
9 files changed, 82 insertions(+), 12 deletions(-)
^ permalink raw reply
* [bpf PATCH] bpf: fix uninitialized variable in bpf tools
From: John Fastabend @ 2018-04-25 22:08 UTC (permalink / raw)
To: ast, daniel, jbenc; +Cc: netdev
Here the variable cont is used as the saved_pointer for a call to
strtok_r(). It is safe to use the value uninitialized in this
context however and the later reference is only ever used if
the strtok_r is successful. But, 'gcc-5' at least doesn't have all
this knowledge so initialize cont to NULL. Additionally, do the
natural NULL check before accessing just for completness.
The warning is the following:
./bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
./bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
} else if (matches(subcmd, "pcap") == 0) {
Fixes: fd981e3c321a "filter: bpf_dbg: add minimal bpf debugger"
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/bpf/bpf_dbg.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpf_dbg.c b/tools/bpf/bpf_dbg.c
index 4f254bc..61b9aa5 100644
--- a/tools/bpf/bpf_dbg.c
+++ b/tools/bpf/bpf_dbg.c
@@ -1063,7 +1063,7 @@ static int cmd_load_pcap(char *file)
static int cmd_load(char *arg)
{
- char *subcmd, *cont, *tmp = strdup(arg);
+ char *subcmd, *cont = NULL, *tmp = strdup(arg);
int ret = CMD_OK;
subcmd = strtok_r(tmp, " ", &cont);
@@ -1073,7 +1073,10 @@ static int cmd_load(char *arg)
bpf_reset();
bpf_reset_breakpoints();
- ret = cmd_load_bpf(cont);
+ if (!cont)
+ ret = CMD_ERR;
+ else
+ ret = cmd_load_bpf(cont);
} else if (matches(subcmd, "pcap") == 0) {
ret = cmd_load_pcap(cont);
} else {
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox