* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Jamal Hadi Salim @ 2016-09-28 11:06 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Eric Dumazet, David Ahern, netdev, linux-kernel, David Miller,
kuznet, jmorris, yoshfuji, kaber, avagin, stephen
In-Reply-To: <20160928105147.GV1876@uranus.lan>
On 16-09-28 06:51 AM, Cyrill Gorcunov wrote:
> On Wed, Sep 28, 2016 at 06:43:01AM -0400, Jamal Hadi Salim wrote:
[..]
>> I dont know how compilation will fail but you may be right with note:
>> that is not how pads have been used in the past. They are supposed to
>> cosmetic annotation which indicates "here's a hole; use it in the
>> future if you are looking to add something". And someone in the
>> future can claim them. I am not sure if MBZ philosophy applies.
>
> This structure is uapi, so anyone has complete rights to reference
> @pad in the userspace programs. Sure it would be more clear to remove
> the @pad completely, but if we choose so I think it's better to do
> on top instead and then if someone complain we can easily revert
> the single trivial commit instead of this big patch.
I am conflicted.
A field labelled "pad" does not appear to be valid as "UAPI". It is
a cosmetic indicator. If you did sizeof() with or without it being
present the value doesnt change.
BTW: There is at least one major structure in inet diag has a hole
today and doesnt have a padding indicator.
> If protocol goes over u8 then complete inet_diag_req_v2 structure will
> have to be reworked becaue @sdiag_protocol is u8 as well. IOW, once
> someone liftup IPPROTO_MAX > 255, he will notice the problem immediately
> because diag for such module simply stop working properly.
>
ok.
cheers,
jamal
^ permalink raw reply
* [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-28 10:52 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
In-Reply-To: <cover.1475048434.git.pabeni@redhat.com>
Avoid usage of common memory accounting functions, since
the logic is pretty much different.
To account for forward allocation, a couple of new atomic_t
members are added to udp_sock: 'mem_alloced' and 'can_reclaim'.
The current forward allocation is estimated as 'mem_alloced'
minus 'sk_rmem_alloc'.
When the forward allocation can't cope with the packet to be
enqueued, 'mem_alloced' is incremented by the packet size
rounded-up to the next SK_MEM_QUANTUM.
After a dequeue, if under memory pressure, we try to partially
reclaim all forward allocated memory rounded down to an
SK_MEM_QUANTUM and 'mem_alloc' is decreased by that amount.
To protect against concurrent reclaim, we use 'can_reclaim' as
an unblocking synchronization point and let only one process
do the work.
sk->sk_forward_alloc is set after each memory update to the
currently estimated forward allocation, without any lock or
protection.
This value is updated/maintained only to expose some
semi-reasonable value to the eventual reader, and is guaranteed
to be 0 at socket destruction time.
The above needs custom memory reclaiming on shutdown, provided
by the udp_destruct_sock() helper, which completely reclaim
the allocated forward memory.
As a consequence of the above schema, enqueue to sk_error_queue
will cause fwd value to be understimated.
v1 -> v2:
- use a udp specific destrctor to perform memory reclaiming
- remove a couple of helpers, unneeded after the above cleanup
- do not reclaim memory on dequeue if not under memory
pressure
- reworked the fwd accounting schema to avoid potential
integer overflow
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/udp.h | 2 +
include/net/udp.h | 7 +++
net/ipv4/udp.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 146 insertions(+)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index d1fd8cd..16aa22b 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -42,6 +42,8 @@ static inline u32 udp_hashfn(const struct net *net, u32 num, u32 mask)
struct udp_sock {
/* inet_sock has to be the first member */
struct inet_sock inet;
+ atomic_t mem_allocated;
+ atomic_t can_reclaim;
#define udp_port_hash inet.sk.__sk_common.skc_u16hashes[0]
#define udp_portaddr_hash inet.sk.__sk_common.skc_u16hashes[1]
#define udp_portaddr_node inet.sk.__sk_common.skc_portaddr_node
diff --git a/include/net/udp.h b/include/net/udp.h
index ea53a87..b9563ef 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -98,6 +98,8 @@ static inline struct udp_hslot *udp_hashslot2(struct udp_table *table,
extern struct proto udp_prot;
extern atomic_long_t udp_memory_allocated;
+extern int udp_memory_pressure;
+extern struct percpu_counter udp_sockets_allocated;
/* sysctl variables for udp */
extern long sysctl_udp_mem[3];
@@ -246,6 +248,10 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
}
/* net/ipv4/udp.c */
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
+int udp_rmem_schedule(struct sock *sk, struct sk_buff *skb);
+void udp_enter_memory_pressure(struct sock *sk);
+
void udp_v4_early_demux(struct sk_buff *skb);
int udp_get_port(struct sock *sk, unsigned short snum,
int (*saddr_cmp)(const struct sock *,
@@ -258,6 +264,7 @@ void udp_flush_pending_frames(struct sock *sk);
void udp4_hwcsum(struct sk_buff *skb, __be32 src, __be32 dst);
int udp_rcv(struct sk_buff *skb);
int udp_ioctl(struct sock *sk, int cmd, unsigned long arg);
+int udp_init_sock(struct sock *sk);
int udp_disconnect(struct sock *sk, int flags);
unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait);
struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 7d96dc2..2218901 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -131,6 +131,12 @@ EXPORT_SYMBOL(sysctl_udp_wmem_min);
atomic_long_t udp_memory_allocated;
EXPORT_SYMBOL(udp_memory_allocated);
+int udp_memory_pressure __read_mostly;
+EXPORT_SYMBOL(udp_memory_pressure);
+
+struct percpu_counter udp_sockets_allocated;
+EXPORT_SYMBOL(udp_sockets_allocated);
+
#define MAX_UDP_PORTS 65536
#define PORTS_PER_CHAIN (MAX_UDP_PORTS / UDP_HTABLE_SIZE_MIN)
@@ -1172,6 +1178,137 @@ out:
return ret;
}
+static int __udp_forward(struct udp_sock *up, int rmem)
+{
+ return atomic_read(&up->mem_allocated) - rmem;
+}
+
+static bool udp_under_memory_pressure(const struct sock *sk)
+{
+ if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
+ mem_cgroup_under_socket_pressure(sk->sk_memcg))
+ return true;
+
+ return READ_ONCE(udp_memory_pressure);
+}
+
+void udp_enter_memory_pressure(struct sock *sk)
+{
+ WRITE_ONCE(udp_memory_pressure, 1);
+}
+EXPORT_SYMBOL(udp_enter_memory_pressure);
+
+/* if partial != 0 do nothing if not under memory pressure and avoid
+ * reclaiming last quanta
+ */
+static void udp_rmem_release(struct sock *sk, int partial)
+{
+ struct udp_sock *up = udp_sk(sk);
+ int fwd, amt;
+
+ if (partial && !udp_under_memory_pressure(sk))
+ return;
+
+ /* we can have concurrent release; if we catch any conflict
+ * we let only one of them do the work
+ */
+ if (atomic_dec_if_positive(&up->can_reclaim) < 0)
+ return;
+
+ fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
+ if (fwd < SK_MEM_QUANTUM + partial) {
+ atomic_inc(&up->can_reclaim);
+ return;
+ }
+
+ amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
+ atomic_sub(amt, &up->mem_allocated);
+ atomic_inc(&up->can_reclaim);
+
+ __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
+ sk->sk_forward_alloc = fwd - amt;
+}
+
+static void udp_rmem_free(struct sk_buff *skb)
+{
+ struct sock *sk = skb->sk;
+
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+ udp_rmem_release(sk, 1);
+}
+
+int udp_rmem_schedule(struct sock *sk, struct sk_buff *skb)
+{
+ int fwd, amt, delta, rmem, err = -ENOMEM;
+ struct udp_sock *up = udp_sk(sk);
+
+ rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
+ if (rmem > sk->sk_rcvbuf)
+ goto drop;
+
+ fwd = __udp_forward(up, rmem);
+ if (fwd > 0)
+ goto no_alloc;
+
+ amt = sk_mem_pages(skb->truesize);
+ delta = amt << SK_MEM_QUANTUM_SHIFT;
+ if (!__sk_mem_raise_allocated(sk, delta, amt, SK_MEM_RECV)) {
+ err = -ENOBUFS;
+ goto drop;
+ }
+
+ /* if we have some skbs in the error queue, the forward allocation could
+ * be understimated, even below 0; avoid exporting such values
+ */
+ fwd = atomic_add_return(delta, &up->mem_allocated) - rmem;
+ if (fwd < 0)
+ fwd = SK_MEM_QUANTUM;
+
+no_alloc:
+ sk->sk_forward_alloc = fwd;
+ skb_orphan(skb);
+ skb->sk = sk;
+ skb->destructor = udp_rmem_free;
+ return 0;
+
+drop:
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+ atomic_inc(&sk->sk_drops);
+ return err;
+}
+EXPORT_SYMBOL_GPL(udp_rmem_schedule);
+
+static void udp_destruct_sock(struct sock *sk)
+{
+ /* reclaim completely the forward allocated memory */
+ __skb_queue_purge(&sk->sk_receive_queue);
+ udp_rmem_release(sk, 0);
+ inet_sock_destruct(sk);
+}
+
+int udp_init_sock(struct sock *sk)
+{
+ struct udp_sock *up = udp_sk(sk);
+
+ atomic_set(&up->mem_allocated, 0);
+ atomic_set(&up->can_reclaim, 1);
+ sk->sk_destruct = udp_destruct_sock;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(udp_init_sock);
+
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len)
+{
+ if (unlikely(READ_ONCE(sk->sk_peek_off) >= 0)) {
+ bool slow = lock_sock_fast(sk);
+
+ sk_peek_offset_bwd(sk, len);
+ unlock_sock_fast(sk, slow);
+ }
+ consume_skb(skb);
+}
+EXPORT_SYMBOL_GPL(skb_consume_udp);
+
/**
* first_packet_length - return length of first packet in receive queue
* @sk: socket
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 1/3] net/socket: factor out helpers for memory and queue manipulation
From: Paolo Abeni @ 2016-09-28 10:52 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
In-Reply-To: <cover.1475048434.git.pabeni@redhat.com>
Basic sock operations that udp code can use with its own
memory accounting schema. No functional change is introduced
in the existing APIs.
v1 -> v2:
- avoid export sock_rmem_free
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/sock.h | 5 +++
net/core/datagram.c | 36 ++++++++++++--------
net/core/sock.c | 96 +++++++++++++++++++++++++++++++++++------------------
3 files changed, 91 insertions(+), 46 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index ebf75db..7e42922 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1274,7 +1274,9 @@ static inline struct inode *SOCK_INODE(struct socket *socket)
/*
* Functions for memory accounting
*/
+int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind);
int __sk_mem_schedule(struct sock *sk, int size, int kind);
+void __sk_mem_reduce_allocated(struct sock *sk, int amount);
void __sk_mem_reclaim(struct sock *sk, int amount);
#define SK_MEM_QUANTUM ((int)PAGE_SIZE)
@@ -1950,6 +1952,9 @@ void sk_reset_timer(struct sock *sk, struct timer_list *timer,
void sk_stop_timer(struct sock *sk, struct timer_list *timer);
+int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb,
+ unsigned int flags);
+void __sock_enqueue_skb(struct sock *sk, struct sk_buff *skb);
int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index b7de71f..bfb973a 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -323,6 +323,27 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len)
}
EXPORT_SYMBOL(__skb_free_datagram_locked);
+int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb,
+ unsigned int flags)
+{
+ int err = 0;
+
+ if (flags & MSG_PEEK) {
+ err = -ENOENT;
+ spin_lock_bh(&sk->sk_receive_queue.lock);
+ if (skb == skb_peek(&sk->sk_receive_queue)) {
+ __skb_unlink(skb, &sk->sk_receive_queue);
+ atomic_dec(&skb->users);
+ err = 0;
+ }
+ spin_unlock_bh(&sk->sk_receive_queue.lock);
+ }
+
+ atomic_inc(&sk->sk_drops);
+ return err;
+}
+EXPORT_SYMBOL(__sk_queue_drop_skb);
+
/**
* skb_kill_datagram - Free a datagram skbuff forcibly
* @sk: socket
@@ -346,23 +367,10 @@ EXPORT_SYMBOL(__skb_free_datagram_locked);
int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
{
- int err = 0;
-
- if (flags & MSG_PEEK) {
- err = -ENOENT;
- spin_lock_bh(&sk->sk_receive_queue.lock);
- if (skb == skb_peek(&sk->sk_receive_queue)) {
- __skb_unlink(skb, &sk->sk_receive_queue);
- atomic_dec(&skb->users);
- err = 0;
- }
- spin_unlock_bh(&sk->sk_receive_queue.lock);
- }
+ int err = __sk_queue_drop_skb(sk, skb, flags);
kfree_skb(skb);
- atomic_inc(&sk->sk_drops);
sk_mem_reclaim_partial(sk);
-
return err;
}
EXPORT_SYMBOL(skb_kill_datagram);
diff --git a/net/core/sock.c b/net/core/sock.c
index 038e660..102f4d1 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -405,24 +405,12 @@ static void sock_disable_timestamp(struct sock *sk, unsigned long flags)
}
-int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+void __sock_enqueue_skb(struct sock *sk, struct sk_buff *skb)
{
unsigned long flags;
struct sk_buff_head *list = &sk->sk_receive_queue;
- if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf) {
- atomic_inc(&sk->sk_drops);
- trace_sock_rcvqueue_full(sk, skb);
- return -ENOMEM;
- }
-
- if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
- atomic_inc(&sk->sk_drops);
- return -ENOBUFS;
- }
-
skb->dev = NULL;
- skb_set_owner_r(skb, sk);
/* we escape from rcu protected region, make sure we dont leak
* a norefcounted dst
@@ -436,6 +424,24 @@ int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_data_ready(sk);
+}
+EXPORT_SYMBOL(__sock_enqueue_skb);
+
+int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+ if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf) {
+ atomic_inc(&sk->sk_drops);
+ trace_sock_rcvqueue_full(sk, skb);
+ return -ENOMEM;
+ }
+
+ if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
+ atomic_inc(&sk->sk_drops);
+ return -ENOBUFS;
+ }
+
+ skb_set_owner_r(skb, sk);
+ __sock_enqueue_skb(sk, skb);
return 0;
}
EXPORT_SYMBOL(__sock_queue_rcv_skb);
@@ -2091,24 +2097,18 @@ int sk_wait_data(struct sock *sk, long *timeo, const struct sk_buff *skb)
EXPORT_SYMBOL(sk_wait_data);
/**
- * __sk_mem_schedule - increase sk_forward_alloc and memory_allocated
+ * __sk_mem_raise_allocated - increase memory_allocated
* @sk: socket
* @size: memory size to allocate
+ * @amt: pages to allocate
* @kind: allocation type
*
- * If kind is SK_MEM_SEND, it means wmem allocation. Otherwise it means
- * rmem allocation. This function assumes that protocols which have
- * memory_pressure use sk_wmem_queued as write buffer accounting.
+ * Similar to __sk_mem_schedule(), but does not update sk_forward_alloc
*/
-int __sk_mem_schedule(struct sock *sk, int size, int kind)
+int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
{
struct proto *prot = sk->sk_prot;
- int amt = sk_mem_pages(size);
- long allocated;
-
- sk->sk_forward_alloc += amt * SK_MEM_QUANTUM;
-
- allocated = sk_memory_allocated_add(sk, amt);
+ long allocated = sk_memory_allocated_add(sk, amt);
if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
!mem_cgroup_charge_skmem(sk->sk_memcg, amt))
@@ -2169,9 +2169,6 @@ suppress_allocation:
trace_sock_exceed_buf_limit(sk, prot, allocated);
- /* Alas. Undo changes. */
- sk->sk_forward_alloc -= amt * SK_MEM_QUANTUM;
-
sk_memory_allocated_sub(sk, amt);
if (mem_cgroup_sockets_enabled && sk->sk_memcg)
@@ -2179,18 +2176,40 @@ suppress_allocation:
return 0;
}
+EXPORT_SYMBOL(__sk_mem_raise_allocated);
+
+/**
+ * __sk_mem_schedule - increase sk_forward_alloc and memory_allocated
+ * @sk: socket
+ * @size: memory size to allocate
+ * @kind: allocation type
+ *
+ * If kind is SK_MEM_SEND, it means wmem allocation. Otherwise it means
+ * rmem allocation. This function assumes that protocols which have
+ * memory_pressure use sk_wmem_queued as write buffer accounting.
+ */
+int __sk_mem_schedule(struct sock *sk, int size, int kind)
+{
+ int ret, amt = sk_mem_pages(size);
+
+ sk->sk_forward_alloc += amt << SK_MEM_QUANTUM_SHIFT;
+ ret = __sk_mem_raise_allocated(sk, size, amt, kind);
+ if (!ret)
+ sk->sk_forward_alloc -= amt << SK_MEM_QUANTUM_SHIFT;
+ return ret;
+}
EXPORT_SYMBOL(__sk_mem_schedule);
/**
- * __sk_mem_reclaim - reclaim memory_allocated
+ * __sk_mem_reduce_allocated - reclaim memory_allocated
* @sk: socket
- * @amount: number of bytes (rounded down to a SK_MEM_QUANTUM multiple)
+ * @amount: number of quanta
+ *
+ * Similar to __sk_mem_reclaim(), but does not update sk_forward_alloc
*/
-void __sk_mem_reclaim(struct sock *sk, int amount)
+void __sk_mem_reduce_allocated(struct sock *sk, int amount)
{
- amount >>= SK_MEM_QUANTUM_SHIFT;
sk_memory_allocated_sub(sk, amount);
- sk->sk_forward_alloc -= amount << SK_MEM_QUANTUM_SHIFT;
if (mem_cgroup_sockets_enabled && sk->sk_memcg)
mem_cgroup_uncharge_skmem(sk->sk_memcg, amount);
@@ -2199,6 +2218,19 @@ void __sk_mem_reclaim(struct sock *sk, int amount)
(sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)))
sk_leave_memory_pressure(sk);
}
+EXPORT_SYMBOL(__sk_mem_reduce_allocated);
+
+/**
+ * __sk_mem_reclaim - reclaim sk_forward_alloc and memory_allocated
+ * @sk: socket
+ * @amount: number of bytes (rounded down to a SK_MEM_QUANTUM multiple)
+ */
+void __sk_mem_reclaim(struct sock *sk, int amount)
+{
+ amount >>= SK_MEM_QUANTUM_SHIFT;
+ sk->sk_forward_alloc -= amount << SK_MEM_QUANTUM_SHIFT;
+ __sk_mem_reduce_allocated(sk, amount);
+}
EXPORT_SYMBOL(__sk_mem_reclaim);
int sk_set_peek_off(struct sock *sk, int val)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 3/3] udp: use it's own memory accounting schema
From: Paolo Abeni @ 2016-09-28 10:52 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1475048434.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Completely avoid default sock memory accounting and replace it
with udp-specific accounting.
The udp code now tracks the memory pressure status, to optimize
memory reclaiming.
Since the new memory accounting model does not require socket
locking, remove the lock on enqueue and free and avoid using the
backlog on enqueue.
Be sure to clean-up rx queue memory on socket destruction, using
udp its own sk_destruct.
Tested using pktgen with random src port, 64 bytes packet,
wire-speed on a 10G link as sender and udp_sink as the receiver,
using an l4 tuple rxhash to stress the contention, and one or more
udp_sink instances with reuseport.
nr readers Kpps (vanilla) Kpps (patched)
1 150 600
3 1300 2300
6 3100 3900
9 4300 4650
12 5800 6550
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- add memory pressure support
- fixed dropwatch accounting for ipv6
Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
net/ipv4/udp.c | 42 +++++++++++++-----------------------------
net/ipv6/udp.c | 33 ++++++++++++---------------------
net/sunrpc/svcsock.c | 20 ++++++++++++++++----
net/sunrpc/xprtsock.c | 2 +-
4 files changed, 42 insertions(+), 55 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 2218901..c05bf5d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1338,13 +1338,8 @@ static int first_packet_length(struct sock *sk)
res = skb ? skb->len : -1;
spin_unlock_bh(&rcvq->lock);
- if (!skb_queue_empty(&list_kill)) {
- bool slow = lock_sock_fast(sk);
-
+ if (!skb_queue_empty(&list_kill))
__skb_queue_purge(&list_kill);
- sk_mem_reclaim_partial(sk);
- unlock_sock_fast(sk, slow);
- }
return res;
}
@@ -1393,7 +1388,6 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
int err;
int is_udplite = IS_UDPLITE(sk);
bool checksum_valid = false;
- bool slow;
if (flags & MSG_ERRQUEUE)
return ip_recv_error(sk, msg, len, addr_len);
@@ -1434,13 +1428,12 @@ try_again:
}
if (unlikely(err)) {
- trace_kfree_skb(skb, udp_recvmsg);
if (!peeked) {
atomic_inc(&sk->sk_drops);
UDP_INC_STATS(sock_net(sk),
UDP_MIB_INERRORS, is_udplite);
}
- skb_free_datagram_locked(sk, skb);
+ kfree_skb(skb);
return err;
}
@@ -1465,16 +1458,15 @@ try_again:
if (flags & MSG_TRUNC)
err = ulen;
- __skb_free_datagram_locked(sk, skb, peeking ? -err : err);
+ skb_consume_udp(sk, skb, peeking ? -err : err);
return err;
csum_copy_err:
- slow = lock_sock_fast(sk);
- if (!skb_kill_datagram(sk, skb, flags)) {
+ if (!__sk_queue_drop_skb(sk, skb, flags)) {
UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
}
- unlock_sock_fast(sk, slow);
+ kfree_skb(skb);
/* starting over for a new packet, but check if we need to yield */
cond_resched();
@@ -1593,7 +1585,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
sk_incoming_cpu_update(sk);
}
- rc = __sock_queue_rcv_skb(sk, skb);
+ rc = udp_rmem_schedule(sk, skb);
if (rc < 0) {
int is_udplite = IS_UDPLITE(sk);
@@ -1607,8 +1599,8 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
return -1;
}
+ __sock_enqueue_skb(sk, skb);
return 0;
-
}
static struct static_key udp_encap_needed __read_mostly;
@@ -1630,7 +1622,6 @@ EXPORT_SYMBOL(udp_encap_enable);
int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
- int rc;
int is_udplite = IS_UDPLITE(sk);
/*
@@ -1723,19 +1714,8 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
goto drop;
}
- rc = 0;
-
ipv4_pktinfo_prepare(sk, skb);
- bh_lock_sock(sk);
- if (!sock_owned_by_user(sk))
- rc = __udp_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
- bh_unlock_sock(sk);
- goto drop;
- }
- bh_unlock_sock(sk);
-
- return rc;
+ return __udp_queue_rcv_skb(sk, skb);
csum_error:
__UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
@@ -2345,19 +2325,22 @@ struct proto udp_prot = {
.connect = ip4_datagram_connect,
.disconnect = udp_disconnect,
.ioctl = udp_ioctl,
+ .init = udp_init_sock,
.destroy = udp_destroy_sock,
.setsockopt = udp_setsockopt,
.getsockopt = udp_getsockopt,
.sendmsg = udp_sendmsg,
.recvmsg = udp_recvmsg,
.sendpage = udp_sendpage,
- .backlog_rcv = __udp_queue_rcv_skb,
.release_cb = ip4_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.rehash = udp_v4_rehash,
.get_port = udp_v4_get_port,
+ .enter_memory_pressure = udp_enter_memory_pressure,
+ .sockets_allocated = &udp_sockets_allocated,
.memory_allocated = &udp_memory_allocated,
+ .memory_pressure = &udp_memory_pressure,
.sysctl_mem = sysctl_udp_mem,
.sysctl_wmem = &sysctl_udp_wmem_min,
.sysctl_rmem = &sysctl_udp_rmem_min,
@@ -2641,6 +2624,7 @@ void __init udp_init(void)
{
unsigned long limit;
+ percpu_counter_init(&udp_sockets_allocated, 0, GFP_KERNEL);
udp_table_init(&udp_table, "UDP");
limit = nr_free_buffer_pages() / 8;
limit = max(limit, 128UL);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9aa7c1c..c88e576 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -334,7 +334,6 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int is_udplite = IS_UDPLITE(sk);
bool checksum_valid = false;
int is_udp4;
- bool slow;
if (flags & MSG_ERRQUEUE)
return ipv6_recv_error(sk, msg, len, addr_len);
@@ -378,7 +377,6 @@ try_again:
goto csum_copy_err;
}
if (unlikely(err)) {
- trace_kfree_skb(skb, udpv6_recvmsg);
if (!peeked) {
atomic_inc(&sk->sk_drops);
if (is_udp4)
@@ -388,7 +386,7 @@ try_again:
UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
is_udplite);
}
- skb_free_datagram_locked(sk, skb);
+ kfree_skb(skb);
return err;
}
if (!peeked) {
@@ -437,12 +435,11 @@ try_again:
if (flags & MSG_TRUNC)
err = ulen;
- __skb_free_datagram_locked(sk, skb, peeking ? -err : err);
+ skb_consume_udp(sk, skb, peeking ? -err : err);
return err;
csum_copy_err:
- slow = lock_sock_fast(sk);
- if (!skb_kill_datagram(sk, skb, flags)) {
+ if (!__sk_queue_drop_skb(sk, skb, flags)) {
if (is_udp4) {
UDP_INC_STATS(sock_net(sk),
UDP_MIB_CSUMERRORS, is_udplite);
@@ -455,7 +452,7 @@ csum_copy_err:
UDP_MIB_INERRORS, is_udplite);
}
}
- unlock_sock_fast(sk, slow);
+ kfree_skb(skb);
/* starting over for a new packet, but check if we need to yield */
cond_resched();
@@ -523,7 +520,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
sk_incoming_cpu_update(sk);
}
- rc = __sock_queue_rcv_skb(sk, skb);
+ rc = udp_rmem_schedule(sk, skb);
if (rc < 0) {
int is_udplite = IS_UDPLITE(sk);
@@ -535,6 +532,8 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
kfree_skb(skb);
return -1;
}
+
+ __sock_enqueue_skb(sk, skb);
return 0;
}
@@ -556,7 +555,6 @@ EXPORT_SYMBOL(udpv6_encap_enable);
int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
- int rc;
int is_udplite = IS_UDPLITE(sk);
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
@@ -630,17 +628,7 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
skb_dst_drop(skb);
- bh_lock_sock(sk);
- rc = 0;
- if (!sock_owned_by_user(sk))
- rc = __udpv6_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
- bh_unlock_sock(sk);
- goto drop;
- }
- bh_unlock_sock(sk);
-
- return rc;
+ return __udpv6_queue_rcv_skb(sk, skb);
csum_error:
__UDP6_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
@@ -1433,18 +1421,21 @@ struct proto udpv6_prot = {
.connect = ip6_datagram_connect,
.disconnect = udp_disconnect,
.ioctl = udp_ioctl,
+ .init = udp_init_sock,
.destroy = udpv6_destroy_sock,
.setsockopt = udpv6_setsockopt,
.getsockopt = udpv6_getsockopt,
.sendmsg = udpv6_sendmsg,
.recvmsg = udpv6_recvmsg,
- .backlog_rcv = __udpv6_queue_rcv_skb,
.release_cb = ip6_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.rehash = udp_v6_rehash,
.get_port = udp_v6_get_port,
+ .enter_memory_pressure = udp_enter_memory_pressure,
+ .sockets_allocated = &udp_sockets_allocated,
.memory_allocated = &udp_memory_allocated,
+ .memory_pressure = &udp_memory_pressure,
.sysctl_mem = sysctl_udp_mem,
.sysctl_wmem = &sysctl_udp_wmem_min,
.sysctl_rmem = &sysctl_udp_rmem_min,
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 57625f6..e2a55dc 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -39,6 +39,7 @@
#include <net/checksum.h>
#include <net/ip.h>
#include <net/ipv6.h>
+#include <net/udp.h>
#include <net/tcp.h>
#include <net/tcp_states.h>
#include <asm/uaccess.h>
@@ -129,6 +130,18 @@ static void svc_release_skb(struct svc_rqst *rqstp)
}
}
+static void svc_release_udp_skb(struct svc_rqst *rqstp)
+{
+ struct sk_buff *skb = rqstp->rq_xprt_ctxt;
+
+ if (skb) {
+ rqstp->rq_xprt_ctxt = NULL;
+
+ dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
+ consume_skb(skb);
+ }
+}
+
union svc_pktinfo_u {
struct in_pktinfo pkti;
struct in6_pktinfo pkti6;
@@ -575,7 +588,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
goto out_free;
}
local_bh_enable();
- skb_free_datagram_locked(svsk->sk_sk, skb);
+ consume_skb(skb);
} else {
/* we can use it in-place */
rqstp->rq_arg.head[0].iov_base = skb->data;
@@ -602,8 +615,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
return len;
out_free:
- trace_kfree_skb(skb, svc_udp_recvfrom);
- skb_free_datagram_locked(svsk->sk_sk, skb);
+ kfree_skb(skb);
return 0;
}
@@ -660,7 +672,7 @@ static struct svc_xprt_ops svc_udp_ops = {
.xpo_create = svc_udp_create,
.xpo_recvfrom = svc_udp_recvfrom,
.xpo_sendto = svc_udp_sendto,
- .xpo_release_rqst = svc_release_skb,
+ .xpo_release_rqst = svc_release_udp_skb,
.xpo_detach = svc_sock_detach,
.xpo_free = svc_sock_free,
.xpo_prep_reply_hdr = svc_udp_prep_reply_hdr,
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index bf16883..8e04a43 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1074,7 +1074,7 @@ static void xs_udp_data_receive(struct sock_xprt *transport)
skb = skb_recv_datagram(sk, 0, 1, &err);
if (skb != NULL) {
xs_udp_data_read_skb(&transport->xprt, sk, skb);
- skb_free_datagram_locked(sk, skb);
+ consume_skb(skb);
continue;
}
if (!test_and_clear_bit(XPRT_SOCK_DATA_READY, &transport->sock_state))
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next v3 0/3] udp: refactor memory accounting
From: Paolo Abeni @ 2016-09-28 10:52 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs-u79uwXL29TY76Z2rM5mHXA
This patch series refactor the udp memory accounting, replacing the
generic implementation with a custom one, in order to remove the needs for
locking the socket on the enqueue and dequeue operations. The socket backlog
usage is dropped, as well.
The first patch factor out core pieces of some queue and memory management
socket helpers, so that they can later be used by the udp memory accounting
functions.
The second patch adds the memory account helpers, without using them.
The third patch replacse the old rx memory accounting path for udp over ipv4 and
udp over ipv6. In kernel UDP users are updated, as well.
The memory accounting schema is described in detail in the individual patch
commit message.
The performance gain depends on the specific scenario; with few flows (and
little contention in the original code) the differences are in the noise range,
while with several flows contending the same socket, the measured speed-up
is relevant (e.g. even over 100% in case of extreme contention)
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- changed slighly the memory accounting schema, we now perform lazy reclaim
- fixed forward_alloc updating issue
- fixed memory counter integer overflows
Paolo Abeni (3):
net/socket: factor out helpers for memory and queue manipulation
udp: implement memory accounting helpers
udp: use it's own memory accounting schema
include/linux/udp.h | 2 +
include/net/sock.h | 5 ++
include/net/udp.h | 7 ++
net/core/datagram.c | 36 ++++++----
net/core/sock.c | 96 ++++++++++++++++++---------
net/ipv4/udp.c | 179 ++++++++++++++++++++++++++++++++++++++++++--------
net/ipv6/udp.c | 33 ++++------
net/sunrpc/svcsock.c | 20 ++++--
net/sunrpc/xprtsock.c | 2 +-
9 files changed, 279 insertions(+), 101 deletions(-)
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Cyrill Gorcunov @ 2016-09-28 10:51 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Eric Dumazet, David Ahern, netdev, linux-kernel, David Miller,
kuznet, jmorris, yoshfuji, kaber, avagin, stephen
In-Reply-To: <8293413c-a81d-f7ff-24f0-8f58ce877116@mojatatu.com>
On Wed, Sep 28, 2016 at 06:43:01AM -0400, Jamal Hadi Salim wrote:
...
> >
> > Someone may have set it to zero explicitly on source level, and the
> > compilation will fail on new kernel then. So no, keeping the name
> > is reasonable.
> >
>
> I dont know how compilation will fail but you may be right with note:
> that is not how pads have been used in the past. They are supposed to
> cosmetic annotation which indicates "here's a hole; use it in the
> future if you are looking to add something". And someone in the
> future can claim them. I am not sure if MBZ philosophy applies.
This structure is uapi, so anyone has complete rights to reference
@pad in the userspace programs. Sure it would be more clear to remove
the @pad completely, but if we choose so I think it's better to do
on top instead and then if someone complain we can easily revert
the single trivial commit instead of this big patch.
>
> > > Should you not just rename it?
> > > Also I notice when things like __raw_v4_lookup() are claiming it is unsigned
> > > short instead of a u8?
> >
> > The protocol is still up to 255 for a while, is it expected that IPPROTO_MAX
> > will be increased in more-less near future? Of course I can drop the idea
> > of using @pad here and switch to some extended reauest but prefer to stick
> > with simplier solution. Hm?
> >
>
> Ok. If i understood correctly it was already unsigned short before your
> patch -so i agree it doesnt matter. Maybe just put a comment to express
> that if ever protocol goes above 255 it wont be sufficient.
If protocol goes over u8 then complete inet_diag_req_v2 structure will
have to be reworked becaue @sdiag_protocol is u8 as well. IOW, once
someone liftup IPPROTO_MAX > 255, he will notice the problem immediately
because diag for such module simply stop working properly.
^ permalink raw reply
* [PATCHv2 net 5/5] sctp: remove the old ttl expires policy
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1475059350.git.lucien.xin@gmail.com>
The prsctp polices include ttl expires policy already, we should remove
the old ttl expires codes, and just adjust the new polices' codes to be
compatible with the old one for users.
This patch is to remove all the old expires codes, and if prsctp polices
are not set, it will still set msg's expires_at and check the expires in
sctp_check_abandoned.
Note that asoc->prsctp_enable is set by default, so users can't feel any
difference even if they use the old expires api in userspace.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 1 -
net/sctp/chunk.c | 32 ++++++++------------------------
net/sctp/output.c | 3 ---
3 files changed, 8 insertions(+), 28 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 49a4d37..2463354 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -530,7 +530,6 @@ struct sctp_datamsg {
/* Did the messenge fail to send? */
int send_error;
u8 send_failed:1,
- can_abandon:1, /* can chunks from this message can be abandoned. */
can_delay; /* should this message be Nagle delayed */
};
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 0a3dbec..8d366fc 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -52,7 +52,6 @@ static void sctp_datamsg_init(struct sctp_datamsg *msg)
atomic_set(&msg->refcnt, 1);
msg->send_failed = 0;
msg->send_error = 0;
- msg->can_abandon = 0;
msg->can_delay = 1;
msg->expires_at = 0;
INIT_LIST_HEAD(&msg->chunks);
@@ -169,20 +168,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
/* Note: Calculate this outside of the loop, so that all fragments
* have the same expiration.
*/
- if (sinfo->sinfo_timetolive) {
- /* sinfo_timetolive is in milliseconds */
+ if (asoc->peer.prsctp_capable && sinfo->sinfo_timetolive &&
+ (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags) ||
+ !SCTP_PR_POLICY(sinfo->sinfo_flags)))
msg->expires_at = jiffies +
msecs_to_jiffies(sinfo->sinfo_timetolive);
- msg->can_abandon = 1;
-
- pr_debug("%s: msg:%p expires_at:%ld jiffies:%ld\n", __func__,
- msg, msg->expires_at, jiffies);
- }
-
- if (asoc->peer.prsctp_capable &&
- SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
- msg->expires_at =
- jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
/* This is the biggest possible DATA chunk that can fit into
* the packet
@@ -340,18 +330,8 @@ errout:
/* Check whether this message has expired. */
int sctp_chunk_abandoned(struct sctp_chunk *chunk)
{
- if (!chunk->asoc->peer.prsctp_capable ||
- !SCTP_PR_POLICY(chunk->sinfo.sinfo_flags)) {
- struct sctp_datamsg *msg = chunk->msg;
-
- if (!msg->can_abandon)
- return 0;
-
- if (time_after(jiffies, msg->expires_at))
- return 1;
-
+ if (!chunk->asoc->peer.prsctp_capable)
return 0;
- }
if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
time_after(jiffies, chunk->msg->expires_at)) {
@@ -364,6 +344,10 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
return 1;
+ } else if (!SCTP_PR_POLICY(chunk->sinfo.sinfo_flags) &&
+ chunk->msg->expires_at &&
+ time_after(jiffies, chunk->msg->expires_at)) {
+ return 1;
}
/* PRIO policy is processed by sendmsg, not here */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index db01620..a5b8dad 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -868,9 +868,6 @@ static void sctp_packet_append_data(struct sctp_packet *packet,
rwnd = 0;
asoc->peer.rwnd = rwnd;
- /* Has been accepted for transmission. */
- if (!asoc->peer.prsctp_capable)
- chunk->msg->can_abandon = 0;
sctp_chunk_assign_tsn(chunk);
sctp_chunk_assign_ssn(chunk);
}
--
2.1.0
^ permalink raw reply related
* [PATCHv2 net 4/5] sctp: change to check peer prsctp_capable when using prsctp polices
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1475059350.git.lucien.xin@gmail.com>
Now before using prsctp polices, sctp uses asoc->prsctp_enable to
check if prsctp is enabled. However asoc->prsctp_enable is set only
means local host support prsctp, sctp should not abandon packet if
peer host doesn't enable prsctp.
So this patch is to use asoc->peer.prsctp_capable to check if prsctp
is enabled on both side, instead of asoc->prsctp_enable, as asoc's
peer.prsctp_capable is set only when local and peer both enable prsctp.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/chunk.c | 4 ++--
net/sctp/outqueue.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 14990e2..0a3dbec 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -179,7 +179,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
msg, msg->expires_at, jiffies);
}
- if (asoc->prsctp_enable &&
+ if (asoc->peer.prsctp_capable &&
SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
msg->expires_at =
jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
@@ -340,7 +340,7 @@ errout:
/* Check whether this message has expired. */
int sctp_chunk_abandoned(struct sctp_chunk *chunk)
{
- if (!chunk->asoc->prsctp_enable ||
+ if (!chunk->asoc->peer.prsctp_capable ||
!SCTP_PR_POLICY(chunk->sinfo.sinfo_flags)) {
struct sctp_datamsg *msg = chunk->msg;
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 17b6fcd..70d45c3e 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -326,7 +326,7 @@ int sctp_outq_tail(struct sctp_outq *q, struct sctp_chunk *chunk, gfp_t gfp)
sctp_chunk_hold(chunk);
sctp_outq_tail_data(q, chunk);
- if (chunk->asoc->prsctp_enable &&
+ if (chunk->asoc->peer.prsctp_capable &&
SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
chunk->asoc->sent_cnt_removable++;
if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED)
@@ -442,7 +442,7 @@ void sctp_prsctp_prune(struct sctp_association *asoc,
{
struct sctp_transport *transport;
- if (!asoc->prsctp_enable || !asoc->sent_cnt_removable)
+ if (!asoc->peer.prsctp_capable || !asoc->sent_cnt_removable)
return;
msg_len = sctp_prsctp_prune_sent(asoc, sinfo,
@@ -1053,7 +1053,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
/* Mark as failed send. */
sctp_chunk_fail(chunk, SCTP_ERROR_INV_STRM);
- if (asoc->prsctp_enable &&
+ if (asoc->peer.prsctp_capable &&
SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
asoc->sent_cnt_removable--;
sctp_chunk_free(chunk);
@@ -1345,7 +1345,7 @@ int sctp_outq_sack(struct sctp_outq *q, struct sctp_chunk *chunk)
tsn = ntohl(tchunk->subh.data_hdr->tsn);
if (TSN_lte(tsn, ctsn)) {
list_del_init(&tchunk->transmitted_list);
- if (asoc->prsctp_enable &&
+ if (asoc->peer.prsctp_capable &&
SCTP_PR_PRIO_ENABLED(chunk->sinfo.sinfo_flags))
asoc->sent_cnt_removable--;
sctp_chunk_free(tchunk);
--
2.1.0
^ permalink raw reply related
* [PATCHv2 net 3/5] sctp: remove prsctp_param from sctp_chunk
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1475059350.git.lucien.xin@gmail.com>
Now sctp uses chunk->prsctp_param to save the prsctp param for all the
prsctp polices, we didn't need to introduce prsctp_param to sctp_chunk.
We can just use chunk->sinfo.sinfo_timetolive for RTX and BUF polices,
and reuse msg->expires_at for TTL policy, as the prsctp polices and old
expires policy are mutual exclusive.
This patch is to remove prsctp_param from sctp_chunk, and reuse msg's
expires_at for TTL and chunk's sinfo.sinfo_timetolive for RTX and BUF
polices.
Note that sctp can't use chunk's sinfo.sinfo_timetolive for TTL policy,
as it needs a u64 variables to save the expires_at time.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 7 -------
net/sctp/chunk.c | 9 +++++++--
net/sctp/outqueue.c | 4 ++--
net/sctp/sm_make_chunk.c | 15 ---------------
4 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 7b937d7..49a4d37 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -606,13 +606,6 @@ struct sctp_chunk {
/* This needs to be recoverable for SCTP_SEND_FAILED events. */
struct sctp_sndrcvinfo sinfo;
- /* We use this field to record param for prsctp policies,
- * for TTL policy, it is the time_to_drop of this chunk,
- * for RTX policy, it is the max_sent_count of this chunk,
- * for PRIO policy, it is the priority of this chunk.
- */
- unsigned long prsctp_param;
-
/* Which association does this belong to? */
struct sctp_association *asoc;
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index a55e547..14990e2 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -179,6 +179,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
msg, msg->expires_at, jiffies);
}
+ if (asoc->prsctp_enable &&
+ SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
+ msg->expires_at =
+ jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
+
/* This is the biggest possible DATA chunk that can fit into
* the packet
*/
@@ -349,14 +354,14 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
}
if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
- time_after(jiffies, chunk->prsctp_param)) {
+ time_after(jiffies, chunk->msg->expires_at)) {
if (chunk->sent_count)
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(TTL)]++;
else
chunk->asoc->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
return 1;
} else if (SCTP_PR_RTX_ENABLED(chunk->sinfo.sinfo_flags) &&
- chunk->sent_count > chunk->prsctp_param) {
+ chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
return 1;
}
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index c0fe0b5..17b6fcd 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -383,7 +383,7 @@ static int sctp_prsctp_prune_sent(struct sctp_association *asoc,
list_for_each_entry_safe(chk, temp, queue, transmitted_list) {
if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
- chk->prsctp_param <= sinfo->sinfo_timetolive)
+ chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
continue;
list_del_init(&chk->transmitted_list);
@@ -418,7 +418,7 @@ static int sctp_prsctp_prune_unsent(struct sctp_association *asoc,
list_for_each_entry_safe(chk, temp, queue, list) {
if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
- chk->prsctp_param <= sinfo->sinfo_timetolive)
+ chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
continue;
list_del_init(&chk->list);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 8c77b87..46ffecc 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -706,20 +706,6 @@ nodata:
return retval;
}
-static void sctp_set_prsctp_policy(struct sctp_chunk *chunk,
- const struct sctp_sndrcvinfo *sinfo)
-{
- if (!chunk->asoc->prsctp_enable)
- return;
-
- if (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags))
- chunk->prsctp_param =
- jiffies + msecs_to_jiffies(sinfo->sinfo_timetolive);
- else if (SCTP_PR_RTX_ENABLED(sinfo->sinfo_flags) ||
- SCTP_PR_PRIO_ENABLED(sinfo->sinfo_flags))
- chunk->prsctp_param = sinfo->sinfo_timetolive;
-}
-
/* Make a DATA chunk for the given association from the provided
* parameters. However, do not populate the data payload.
*/
@@ -753,7 +739,6 @@ struct sctp_chunk *sctp_make_datafrag_empty(struct sctp_association *asoc,
retval->subh.data_hdr = sctp_addto_chunk(retval, sizeof(dp), &dp);
memcpy(&retval->sinfo, sinfo, sizeof(struct sctp_sndrcvinfo));
- sctp_set_prsctp_policy(retval, sinfo);
nodata:
return retval;
--
2.1.0
^ permalink raw reply related
* [PATCHv2 net 1/5] sctp: move sent_count to the memory hole in sctp_chunk
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1475059350.git.lucien.xin@gmail.com>
Now pahole sctp_chunk, it has 2 memory holes:
struct sctp_chunk {
struct list_head list;
atomic_t refcnt;
/* XXX 4 bytes hole, try to pack */
...
long unsigned int prsctp_param;
int sent_count;
/* XXX 4 bytes hole, try to pack */
This patch is to move up sent_count to fill the 1st one and eliminate
the 2nd one.
It's not just another struct compaction, it also fixes the "netperf-
Throughput_Mbps -37.2% regression" issue when overloading the CPU.
Fixes: a6c2f792873a ("sctp: implement prsctp TTL policy")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index ce93c4b..4f097f5 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -554,6 +554,9 @@ struct sctp_chunk {
atomic_t refcnt;
+ /* How many times this chunk have been sent, for prsctp RTX policy */
+ int sent_count;
+
/* This is our link to the per-transport transmitted list. */
struct list_head transmitted_list;
@@ -610,9 +613,6 @@ struct sctp_chunk {
*/
unsigned long prsctp_param;
- /* How many times this chunk have been sent, for prsctp RTX policy */
- int sent_count;
-
/* Which association does this belong to? */
struct sctp_association *asoc;
--
2.1.0
^ permalink raw reply related
* [PATCHv2 net 2/5] sctp: reuse sent_count to avoid retransmitted chunks for RTT measurements
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
In-Reply-To: <cover.1475059350.git.lucien.xin@gmail.com>
Now sctp uses chunk->resent to record if a chunk is retransmitted, for
RTT measurements with retransmitted DATA chunks. chunk->sent_count was
introduced to record how many times one chunk has been sent for prsctp
RTX policy before. We actually can know if one chunk is retransmitted
by checking chunk->sent_count is greater than 1.
This patch is to remove resent from sctp_chunk and reuse sent_count
to avoid retransmitted chunks for RTT measurements.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 2 +-
net/sctp/output.c | 3 ++-
net/sctp/outqueue.c | 4 +---
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 4f097f5..7b937d7 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -647,7 +647,6 @@ struct sctp_chunk {
#define SCTP_NEED_FRTX 0x1
#define SCTP_DONT_FRTX 0x2
__u16 rtt_in_progress:1, /* This chunk used for RTT calc? */
- resent:1, /* Has this chunk ever been resent. */
has_tsn:1, /* Does this chunk have a TSN yet? */
has_ssn:1, /* Does this chunk have a SSN yet? */
singleton:1, /* Only chunk in the packet? */
@@ -662,6 +661,7 @@ struct sctp_chunk {
fast_retransmit:2; /* Is this chunk fast retransmitted? */
};
+#define sctp_chunk_retransmitted(chunk) (chunk->sent_count > 1)
void sctp_chunk_hold(struct sctp_chunk *);
void sctp_chunk_put(struct sctp_chunk *);
int sctp_user_addto_chunk(struct sctp_chunk *chunk, int len,
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 31b7bc3..db01620 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -547,7 +547,8 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
* for a given destination transport address.
*/
- if (!chunk->resent && !tp->rto_pending) {
+ if (!sctp_chunk_retransmitted(chunk) &&
+ !tp->rto_pending) {
chunk->rtt_in_progress = 1;
tp->rto_pending = 1;
}
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 72e54a4..c0fe0b5 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -536,8 +536,6 @@ void sctp_retransmit_mark(struct sctp_outq *q,
transport->rto_pending = 0;
}
- chunk->resent = 1;
-
/* Move the chunk to the retransmit queue. The chunks
* on the retransmit queue are always kept in order.
*/
@@ -1467,7 +1465,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
* instance).
*/
if (!tchunk->tsn_gap_acked &&
- !tchunk->resent &&
+ !sctp_chunk_retransmitted(tchunk) &&
tchunk->rtt_in_progress) {
tchunk->rtt_in_progress = 0;
rtt = jiffies - tchunk->sent_at;
--
2.1.0
^ permalink raw reply related
* [PATCHv2 net 0/5] sctp: some fixes of prsctp polices
From: Xin Long @ 2016-09-28 10:46 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
This patchset is to improve some codes about prsctp polices, and also
to fix some issues.
v1->v2:
- wrap the check of chunk->sent_count in a macro:
sctp_chunk_retransmitted in patch 2/5.
Xin Long (5):
sctp: move sent_count to the memory hole in sctp_chunk
sctp: reuse sent_count to avoid retransmitted chunks for RTT
measurements
sctp: remove prsctp_param from sctp_chunk
sctp: change to check peer prsctp_capable when using prsctp polices
sctp: remove the old ttl expires policy
include/net/sctp/structs.h | 16 ++++------------
net/sctp/chunk.c | 31 ++++++++++---------------------
net/sctp/output.c | 6 ++----
net/sctp/outqueue.c | 16 +++++++---------
net/sctp/sm_make_chunk.c | 15 ---------------
5 files changed, 23 insertions(+), 61 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: Explaining RX-stages for XDP
From: Jesper Dangaard Brouer via iovisor-dev @ 2016-09-28 10:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eric Dumazet, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iovisor-dev-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org,
John Fastabend, Jamal Hadi Salim, David Miller, Saeed Mahameed,
Edward Cree, Tom Herbert, Daniel Borkmann, Mel Gorman,
Pablo Neira Ayuso
In-Reply-To: <20160928021242.GA77695-+o4/htvd0TDFYCXBM6kdu7fOX0fSgVTm@public.gmane.org>
On Tue, 27 Sep 2016 19:12:44 -0700 Alexei Starovoitov <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Sep 27, 2016 at 11:32:37AM +0200, Jesper Dangaard Brouer wrote:
> >
> > Let me try in a calm way (not like [1]) to explain how I imagine that
> > the XDP processing RX-stage should be implemented. As I've pointed out
> > before[2], I'm proposing splitting up the driver into RX-stages. This
> > is a mental-model change, I hope you can follow my "inception" attempt.
> >
> > The basic concept behind this idea is, if the RX-ring contains
> > multiple "ready" packets, then the kernel was too slow, processing
> > incoming packets. Thus, switch into more efficient mode, which is a
> > "packet-vector" mode.
> >
> > Today, our XDP micro-benchmarks looks amazing, and they are! But once
> > real-life intermixed traffic is used, then we loose the XDP I-cache
> > benefit. XDP is meant for DoS protection, and an attacker can easily
> > construct intermixed traffic. Why not fix this architecturally?
> >
> > Most importantly concept: If XDP return XDP_PASS, do NOT pass the
> > packet up the network stack immediately (that would flush I-cache).
> > Instead store the packet for the next RX-stage. Basically splitting
> > the packet-vector into two packet-vectors, one for network-stack and
> > one for XDP. Thus, intermixed XDP vs. netstack not longer have effect
> > on XDP performance.
> >
> > The reason for also creating an XDP packet-vector, is to move the
> > XDP_TX transmit code out of the XDP processing stage (and future
> > features). This maximize I-cache availability to the eBPF program,
> > and make eBPF performance more uniform across drivers.
> >
> >
> > Inception:
> > * Instead of individual packets, see it as a RX packet-vector.
> > * XDP should be seen as a stage *before* the network stack gets called.
> >
> > If your mind can handle it: I'm NOT proposing a RX-vector of 64-packets.
> > I actually want N-packet per vector (8-16). As the NIC HW RX process
> > runs concurrently, and by the time it takes to process N-packets, more
> > packets have had a chance to arrive in the RX-ring queue.
>
> Sounds like what Edward was proposing earlier with building
> link list of skbs and passing further into stack?
> Or the idea is different ?
The idea is quite different. It has nothing to do with Edward's
proposal[3]. The RX packet-vector is simply an array, either of pointers
or index numbers (into the RX-ring). The needed changes are completely
contained inside the driver.
> As far as intermixed XDP vs stack traffic, I think for DoS case the
> traffic patterns are binary. Either all of it is good or under attack
> most of the traffic is bad, so makes sense to optimize for these two.
> 50/50 case I think is artificial and not worth optimizing for.
Sorry, but I feel you have completely misunderstood the concept of my
idea. It does not matter what traffic pattern you believe or don't
believe in, it is irrelevant. The fact is that intermixed traffic is
possible with the current solution. The core of my idea is to remove
the possibility for this intermixed traffic to occur, simply by seeing
XDP as a RX-stage before the stack.
> For all good traffic whether xdp is there or not shouldn't matter
> for this N-vector optimization. Whether it's a batch of 8, 16 or 64,
> either via link-list or array, it should probably be a generic
> mechanism independent of any xdp stuff.
I also feel you have misunderstood the N-vector "optimization".
But, yes, this introduction of RX-stages is independent of XDP.
The RX-stages is a generic change to the drivers programming model.
[...]
> I think existing mlx4+xdp is already optimized for 'mostly attack' traffic
> and performs pretty well, since imo 'all drop' benchmark is accurate.
The "all drop" benchmark is as artificial as it gets. It think Eric
agrees.
My idea is confining the XDP_DROP part to a RX-stage _before_ the
netstack. Then, the "all drop" benchmark number will get a little more
trustworthy.
> Optimizing xdp for 'mostly good' traffic is indeed a challange.
> We'd need all the tricks to make it as good as normal skb-based traffic.
>
> I haven't seen any tests yet comparing xdp with 'return XDP_PASS' program
> vs no xdp at all running netperf tcp/udp in user space. It shouldn't
> be too far off.
Well, I did post numbers to the list with a 'return XDP_PASS' program[4]:
https://mid.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg122350.html
Wake-up and smell the coffee, please revise your assumptions:
* It showed that the performance reduction is 25.98%!!!
(AB comparison dropping packets in iptables raw)
Conclusion: These measurements confirm that we need a page recycle
facility for the drivers before switching to order-0 allocations.
I did the same kind of experiment with mlx5. Where I change the memory
model to order-0 pages, and then I implemented page_pool on top. (Below
number are before I implemented the DMA part of page_pool, which do
work now).
page_pool work
- iptables-raw-drop: driver mlx5
* 4,487,518 pps - baseline-before => 100.0%
* 3,624,237 pps - mlx5 order0-patch => - 19.2% (slower)
* 4,806,142 pps - PoC page_pool patch => +7.1% (faster)
This Prove-of-Concept page_pool patch show that it is worth doing, as
the end result is a 7% performance improvement. It also show page
recycling is definitely needed, as this also show that the cost
switching to order-0 is approx 212 cycles on this machine.
(1/4487518-1/3624237)*10^9* 4GHz = -212.32 cycles (cost change to order-0)
(1/4806142-1/3624237)*10^9* 4GHz = -271.41 cycles (gain of recycling)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
[1] https://mid.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg127043.html
[2] http://lists.openwall.net/netdev/2016/01/15/51
[3] http://lists.openwall.net/netdev/2016/04/19/89
[4] https://mid.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg122350.html
^ permalink raw reply
* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Jamal Hadi Salim @ 2016-09-28 10:43 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Eric Dumazet, David Ahern, netdev, linux-kernel, David Miller,
kuznet, jmorris, yoshfuji, kaber, avagin, stephen
In-Reply-To: <20160928101726.GU1876@uranus.lan>
On 16-09-28 06:17 AM, Cyrill Gorcunov wrote:
> On Wed, Sep 28, 2016 at 06:08:00AM -0400, Jamal Hadi Salim wrote:
> ...
>>> @@ -38,7 +38,10 @@ struct inet_diag_req_v2 {
>>> __u8 sdiag_family;
>>> __u8 sdiag_protocol;
>>> __u8 idiag_ext;
>>> - __u8 pad;
>>> + union {
>>> + __u8 pad;
>>> + __u8 sdiag_raw_protocol; /* SOCK_RAW only, @pad for others */
>>> + };
>>
>>
>> Above looks funny. Why is it a union? pad is for exposing a byte-hole
>> for padding/alignment reasons and i doubt anybody is using it.
>
> Someone may have set it to zero explicitly on source level, and the
> compilation will fail on new kernel then. So no, keeping the name
> is reasonable.
>
I dont know how compilation will fail but you may be right with note:
that is not how pads have been used in the past. They are supposed to
cosmetic annotation which indicates "here's a hole; use it in the
future if you are looking to add something". And someone in the
future can claim them. I am not sure if MBZ philosophy applies.
>> Should you not just rename it?
>> Also I notice when things like __raw_v4_lookup() are claiming it is unsigned
>> short instead of a u8?
>
> The protocol is still up to 255 for a while, is it expected that IPPROTO_MAX
> will be increased in more-less near future? Of course I can drop the idea
> of using @pad here and switch to some extended reauest but prefer to stick
> with simplier solution. Hm?
>
Ok. If i understood correctly it was already unsigned short before your
patch -so i agree it doesnt matter. Maybe just put a comment to express
that if ever protocol goes above 255 it wont be sufficient.
cheers,
jamal
PS:- sorry for butting in the discussion - i blame it on coffee.
^ permalink raw reply
* Re: [PATCH v2 net] net: skbuff: skb_vlan_push: Fix wrong unwinding of skb->data after __vlan_insert_tag call
From: Daniel Borkmann @ 2016-09-28 10:30 UTC (permalink / raw)
To: Shmulik Ladkani, David S. Miller, Pravin Shelar
Cc: netdev, Shmulik Ladkani, Jiri Pirko
In-Reply-To: <1475053721-27676-1-git-send-email-shmulik.ladkani@gmail.com>
On 09/28/2016 11:08 AM, Shmulik Ladkani wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
>
> In case 'skb_vlan_push' is called on an skb with a hw-accel vlan tag
> present, the existing hw-accel tag is inserted into the payload, and
> the new given tag is placed as new hw-accel tag.
>
> In order to insert the existing hw-accel tag, 'skb_vlan_push' adjusts
> the 'data' pointer at the mac_header (if needed), invokes __vlan_insert_tag,
> and finally re-adjusts 'data' back to its original position (according
> to the remembered "adjustment offset").
>
> However, successful '__vlan_insert_tag' pushes 4 more bytes at start of
> frame.
> Alas, the remembered "adjustment offset" is NOT fixed to account for
> these additional 4 bytes, so the subsequent '__skb_pull(skb, offset)'
> fails to unwind 'data' back to its original location.
>
> Since 'skb->mac_len' IS fixed to account for the additional 4 bytes
> (incremented to a total of 18 bytes), any access to
> 'skb->data - skb->mac_len' points to bytes PRIOR start of frame.
>
> This is problematic, as many constructs in the stack are issuing
> 'skb_push(skb, skb->mac_len)' prior xmit to a device (e.g tcf_mirred,
> tcf_bpf, nf_dup_netdev_egress), resulting in bogus frames being
> xmitted (having random 4 bytes at start of frame).
>
> For example:
>
> # ip l add dev d0 type dummy
> # tc filter add dev eth0 parent ffff: pref 1 basic \
> action vlan push protocol 802.1ad id 5 pipe \
> action mirred egress redirect dev d0
>
> Any 802.1q (hw-accel) tagged frames arriving on eth0 are xmitted as
> bogus frames on d0; whereas the expected behavior is having QinQ frames.
>
> Fix, by properly accouting the additionally pushed 4 bytes (in the case
> where an adjustment to point at mac_header was done).
>
> Fixes: 93515d53b1 ("net: move vlan pop/push functions into common code")
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Cc: Pravin Shelar <pshelar@ovn.org>
> Cc: Jiri Pirko <jiri@mellanox.com>
> ---
> v2: Instead of reducing mac_len by 4 bytes, which was found incorrect,
> fix the problem of wrong unwinding of 'skb->data'
>
> David, if patch ok, suggest this goes to -stable
>
> net/core/skbuff.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d36c754..3926b79 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4608,6 +4608,8 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
>
> skb->protocol = skb->vlan_proto;
> skb->mac_len += VLAN_HLEN;
> + if (offset)
> + offset += VLAN_HLEN;
>
> skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
> __skb_pull(skb, offset);
This looks much better indeed than your v1 of this patch.
v1 would have definitely changed existing behavior for the cls_bpf/act_bpf
case. Both start at the beginning of mac header from ingress and egress side.
So when frame comes in on ingress, skb->data points to start of net header,
we then do __skb_push(skb, skb->mac_len) before running BPF prog, and after
return from BPF prog again __skb_pull(skb, skb->mac_len) to return to original
location. Thus in skb_vlan_push() from BPF helper call offset is always 0;
perhaps similar in ovs case.
With the removed skb->mac_len adjustment in skb_vlan_push() from your v1, we
would then have pointed into vlan header on return to stack instead of net
header location as we do currently.
So the issue might only be visible to act_vlan as the other remaining user of
skb_vlan_push(). Above fix looks better to me. So if we don't start at the
mac header yet, we need to do the __skb_push()/__skb_pull() adjustment from
there, and since we expand mac header, we need to take these 4 bytes into
account as well for returning to original location. My only question would
be: what about __skb_vlan_pop(), wouldn't that then need the same adjustment
a la offset -= VLAN_HLEN?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH RFC 1/6] net: dsa: mv88e6xxx: Implement interrupt support.
From: Andrew Lunn @ 2016-09-28 10:22 UTC (permalink / raw)
To: Florian Fainelli, Vivien Didelot; +Cc: netdev
In-Reply-To: <1475051544-18561-2-git-send-email-andrew@lunn.ch>
The cover email got rejected, because i used MV88E6XXX in the subject
line, and it did not like the XXX.
----------------------------------------------------------------------
>From 838e38d8079341ffdeb98c3c1bf043627c24cfc2 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 28 Sep 2016 03:23:04 -0500
Subject: [PATCH RFC 0/6] Interrupt support for MV88E6XYZ
NOT FOR MERGING
This RFC patchset add interrupt controller support to the MV88E6XXX.
This allows access to the interrupts the internal PHY generate. These
interrupts can then be associated to a PHY device in the device tree
and used by the PHY lib, rather than polling.
An earlier version of these patches were posted many moons ago. They
have been rebased onto net-next and adapted to the new structure of
the driver. However, still outstanding is if this is the correct way
to implement this feature, via an IRQ domain made available in the
device tree, and then using the standard PHY binding. This was
questioned last time, and never really resolved.
Comment welcome.
Andrew
Andrew Lunn (6):
net: dsa: mv88e6xxx: Implement interrupt support.
net: phy: Use threaded IRQ, to allow IRQ from sleeping devices
net: phy: Threaded interrupts allow some simplification
net: phy: Use phy name when requesting the interrupt
net: phy: Trigger state machine on state change and not polling.
arm: vf610: zii devel b: Add support for switch interrupts
.../devicetree/bindings/net/dsa/marvell.txt | 21 +-
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 51 +++++
drivers/net/dsa/mv88e6xxx/chip.c | 247 ++++++++++++++++++++-
drivers/net/dsa/mv88e6xxx/global2.c | 140 +++++++++++-
drivers/net/dsa/mv88e6xxx/global2.h | 11 +
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 32 ++-
drivers/net/phy/phy.c | 66 +++---
drivers/net/phy/phy_device.c | 1 -
include/linux/phy.h | 4 +-
9 files changed, 529 insertions(+), 44 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Cyrill Gorcunov @ 2016-09-28 10:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Eric Dumazet, David Ahern, netdev, linux-kernel, David Miller,
kuznet, jmorris, yoshfuji, kaber, avagin, stephen
In-Reply-To: <caef7649-9d25-6429-eeab-96c027d6cbc2@mojatatu.com>
On Wed, Sep 28, 2016 at 06:08:00AM -0400, Jamal Hadi Salim wrote:
...
> > @@ -38,7 +38,10 @@ struct inet_diag_req_v2 {
> > __u8 sdiag_family;
> > __u8 sdiag_protocol;
> > __u8 idiag_ext;
> > - __u8 pad;
> > + union {
> > + __u8 pad;
> > + __u8 sdiag_raw_protocol; /* SOCK_RAW only, @pad for others */
> > + };
>
>
> Above looks funny. Why is it a union? pad is for exposing a byte-hole
> for padding/alignment reasons and i doubt anybody is using it.
Someone may have set it to zero explicitly on source level, and the
compilation will fail on new kernel then. So no, keeping the name
is reasonable.
> Should you not just rename it?
> Also I notice when things like __raw_v4_lookup() are claiming it is unsigned
> short instead of a u8?
The protocol is still up to 255 for a while, is it expected that IPPROTO_MAX
will be increased in more-less near future? Of course I can drop the idea
of using @pad here and switch to some extended reauest but prefer to stick
with simplier solution. Hm?
^ permalink raw reply
* Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Jamal Hadi Salim @ 2016-09-28 10:08 UTC (permalink / raw)
To: Cyrill Gorcunov, Eric Dumazet, David Ahern
Cc: netdev, linux-kernel, David Miller, kuznet, jmorris, yoshfuji,
kaber, avagin, stephen
In-Reply-To: <20160928090357.GT1876@uranus.lan>
On 16-09-28 05:03 AM, Cyrill Gorcunov wrote:
> In criu we are actively using diag interface to collect sockets
> present in the system when dumping applications. And while for
> unix, tcp, udp[lite], packet, netlink it works as expected,
> the raw sockets do not have. Thus add it.
>
> v2:
> - add missing sock_put calls in raw_diag_dump_one (by eric.dumazet@)
> - implement @destroy for diag requests (by dsa@)
>
> v3:
> - add export of raw_abort for IPv6 (by dsa@)
> - pass net-admin flag into inet_sk_diag_fill due to
> changes in net-next branch (by dsa@)
>
> v4:
> - use @pad in struct inet_diag_req_v2 for raw socket
> protocol specification: raw module carries sockets
> which may have custom protocol passed from socket()
> syscall and sole @sdiag_protocol is not enough to
> match underlied ones
> - start reporting protocol specifed in socket() call
> when sockets are raw ones for the same reason: user
> space tools like ss may parse this attribute and use
> it for socket matching
>
> v5 (by eric.dumazet@):
> - use sock_hold in raw_sock_get instead of atomic_inc,
> we're holding (raw_v4_hashinfo|raw_v6_hashinfo)->lock
> when looking up so counter won't be zero here.
>
> CC: David S. Miller <davem@davemloft.net>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> CC: David Ahern <dsa@cumulusnetworks.com>
> CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> CC: James Morris <jmorris@namei.org>
> CC: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> CC: Patrick McHardy <kaber@trash.net>
> CC: Andrey Vagin <avagin@openvz.org>
> CC: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>
> Thanks all for feedback! Take a look please once time permit.
>
> include/net/raw.h | 6 +
> include/net/rawv6.h | 7 +
> include/uapi/linux/inet_diag.h | 5
> net/ipv4/Kconfig | 8 +
> net/ipv4/Makefile | 1
> net/ipv4/inet_diag.c | 9 +
> net/ipv4/raw.c | 21 +++
> net/ipv4/raw_diag.c | 233 +++++++++++++++++++++++++++++++++++++++++
> net/ipv6/raw.c | 7 -
> 9 files changed, 292 insertions(+), 5 deletions(-)
>
> Index: linux-ml.git/include/net/raw.h
> ===================================================================
> --- linux-ml.git.orig/include/net/raw.h
> +++ linux-ml.git/include/net/raw.h
> @@ -23,6 +23,12 @@
>
> extern struct proto raw_prot;
>
> +extern struct raw_hashinfo raw_v4_hashinfo;
> +struct sock *__raw_v4_lookup(struct net *net, struct sock *sk,
> + unsigned short num, __be32 raddr,
> + __be32 laddr, int dif);
> +
> +int raw_abort(struct sock *sk, int err);
> void raw_icmp_error(struct sk_buff *, int, u32);
> int raw_local_deliver(struct sk_buff *, int);
>
> Index: linux-ml.git/include/net/rawv6.h
> ===================================================================
> --- linux-ml.git.orig/include/net/rawv6.h
> +++ linux-ml.git/include/net/rawv6.h
> @@ -3,6 +3,13 @@
>
> #include <net/protocol.h>
>
> +extern struct raw_hashinfo raw_v6_hashinfo;
> +struct sock *__raw_v6_lookup(struct net *net, struct sock *sk,
> + unsigned short num, const struct in6_addr *loc_addr,
> + const struct in6_addr *rmt_addr, int dif);
> +
> +int raw_abort(struct sock *sk, int err);
> +
> void raw6_icmp_error(struct sk_buff *, int nexthdr,
> u8 type, u8 code, int inner_offset, __be32);
> bool raw6_local_deliver(struct sk_buff *, int);
> Index: linux-ml.git/include/uapi/linux/inet_diag.h
> ===================================================================
> --- linux-ml.git.orig/include/uapi/linux/inet_diag.h
> +++ linux-ml.git/include/uapi/linux/inet_diag.h
> @@ -38,7 +38,10 @@ struct inet_diag_req_v2 {
> __u8 sdiag_family;
> __u8 sdiag_protocol;
> __u8 idiag_ext;
> - __u8 pad;
> + union {
> + __u8 pad;
> + __u8 sdiag_raw_protocol; /* SOCK_RAW only, @pad for others */
> + };
Above looks funny. Why is it a union? pad is for exposing a byte-hole
for padding/alignment reasons and i doubt anybody is using it.
Should you not just rename it?
Also I notice when things like __raw_v4_lookup() are claiming it is
unsigned short instead of a u8?
cheers,
jamal
^ permalink raw reply
* Re: ath6kl: fix return value in ath6kl_wmi_set_pvb_cmd
From: Kalle Valo @ 2016-09-28 10:01 UTC (permalink / raw)
To: Chaehyun Lim; +Cc: linux-wireless, netdev, Chaehyun Lim
In-Reply-To: <20160918063024.2535-1-chaehyun.lim@gmail.com>
Chaehyun Lim <chaehyun.lim@gmail.com> wrote:
> When building with W=1, we got one warning as belows:
> drivers/net/wireless/ath/ath6kl/wmi.c:3509:6: warning: variable ‘ret’
> set but not used [-Wunused-but-set-variable]
>
> At the end of ath6kl_wmi_set_pvb_cmd, it is returned by 0 regardless of
> return value of ath6kl_wmi_cmd_send.
> This patch fixes return value from 0 to ret that has result of
> ath6kl_wmi_cmd_send execution.
>
> Signed-off-by: Chaehyun Lim <chaehyun.lim@gmail.com>
Patch applied to ath-next branch of ath.git, thanks.
b93015057e31 ath6kl: fix return value in ath6kl_wmi_set_pvb_cmd
--
https://patchwork.kernel.org/patch/9337611/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [RFC v2 2/4] staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid conflicts with LED namespace
From: Greg KH @ 2016-09-28 9:27 UTC (permalink / raw)
To: Zach Brown
Cc: devel, florian.c.schilhabel, f.fainelli, netdev, linux-kernel,
mlindner, Larry.Finger
In-Reply-To: <1475013111-18942-3-git-send-email-zach.brown@ni.com>
On Tue, Sep 27, 2016 at 04:51:49PM -0500, Zach Brown wrote:
> Adding led support for phy causes namespace conflicts for some
> phy drivers.
>
> The rtl871 driver declared an enum for representing LED states. The enum
> contains constant LED_OFF which conflicted with declaration found in
> linux/leds.h. LED_OFF changed to LED_STATE_OFF
> In order to avoid a possible future collision LED_ON was changed to
> LED_STATE_ON as well.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
> drivers/staging/rtl8712/rtl8712_led.c | 388 +++++++++++++++++-----------------
> 1 file changed, 194 insertions(+), 194 deletions(-)
Ick, messy. I'll be glad to take this patch now to make your life
easier in the future.
thanks,
greg k-h
^ permalink raw reply
* RE: [RFC PATCH net-next 2/2] sfc: report 4-tuple UDP hashing to ethtool, if it's enabled
From: David Laight @ 2016-09-28 9:12 UTC (permalink / raw)
To: 'Edward Cree', linux-net-drivers@solarflare.com,
netdev@vger.kernel.org, davem@davemloft.net
Cc: bkenward@solarflare.com
In-Reply-To: <8341c601-674a-74ff-c6dd-689c19b3ce7f@solarflare.com>
From: Edward Cree
> Sent: 27 September 2016 17:36
...
> + case UDP_V4_FLOW:
> + if (efx->rx_hash_udp_4tuple)
> + /* fall through */
> + /* else fall further! */
If you invert the above and add a goto...
if (!efx->rx_hash_udp_4tuple)
goto set_ip;
> case TCP_V4_FLOW:
> - info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
> /* fall through */
> - case UDP_V4_FLOW:
> case SCTP_V4_FLOW:
> case AH_ESP_V4_FLOW:
> case IPV4_FLOW:
set_ip:
> info->data |= RXH_IP_SRC | RXH_IP_DST;
> min_revision = EFX_REV_FALCON_B0;
> break;
It might look better.
David
^ permalink raw reply
* [PATCH v2 net] net: skbuff: skb_vlan_push: Fix wrong unwinding of skb->data after __vlan_insert_tag call
From: Shmulik Ladkani @ 2016-09-28 9:08 UTC (permalink / raw)
To: David S. Miller, Pravin Shelar
Cc: Daniel Borkmann, netdev, Shmulik Ladkani, Jiri Pirko
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
In case 'skb_vlan_push' is called on an skb with a hw-accel vlan tag
present, the existing hw-accel tag is inserted into the payload, and
the new given tag is placed as new hw-accel tag.
In order to insert the existing hw-accel tag, 'skb_vlan_push' adjusts
the 'data' pointer at the mac_header (if needed), invokes __vlan_insert_tag,
and finally re-adjusts 'data' back to its original position (according
to the remembered "adjustment offset").
However, successful '__vlan_insert_tag' pushes 4 more bytes at start of
frame.
Alas, the remembered "adjustment offset" is NOT fixed to account for
these additional 4 bytes, so the subsequent '__skb_pull(skb, offset)'
fails to unwind 'data' back to its original location.
Since 'skb->mac_len' IS fixed to account for the additional 4 bytes
(incremented to a total of 18 bytes), any access to
'skb->data - skb->mac_len' points to bytes PRIOR start of frame.
This is problematic, as many constructs in the stack are issuing
'skb_push(skb, skb->mac_len)' prior xmit to a device (e.g tcf_mirred,
tcf_bpf, nf_dup_netdev_egress), resulting in bogus frames being
xmitted (having random 4 bytes at start of frame).
For example:
# ip l add dev d0 type dummy
# tc filter add dev eth0 parent ffff: pref 1 basic \
action vlan push protocol 802.1ad id 5 pipe \
action mirred egress redirect dev d0
Any 802.1q (hw-accel) tagged frames arriving on eth0 are xmitted as
bogus frames on d0; whereas the expected behavior is having QinQ frames.
Fix, by properly accouting the additionally pushed 4 bytes (in the case
where an adjustment to point at mac_header was done).
Fixes: 93515d53b1 ("net: move vlan pop/push functions into common code")
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Pravin Shelar <pshelar@ovn.org>
Cc: Jiri Pirko <jiri@mellanox.com>
---
v2: Instead of reducing mac_len by 4 bytes, which was found incorrect,
fix the problem of wrong unwinding of 'skb->data'
David, if patch ok, suggest this goes to -stable
net/core/skbuff.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d36c754..3926b79 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4608,6 +4608,8 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
skb->protocol = skb->vlan_proto;
skb->mac_len += VLAN_HLEN;
+ if (offset)
+ offset += VLAN_HLEN;
skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
__skb_pull(skb, offset);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v5 0/7] Reduce cache miss for snmp_fold_field
From: David Miller @ 2016-09-28 9:08 UTC (permalink / raw)
To: hejianet
Cc: netdev, linux-sctp, linux-kernel, kuznet, jmorris, yoshfuji,
kaber, vyasevich, nhorman, steffen.klassert, herbert,
marcelo.leitner
In-Reply-To: <1475043748-18161-1-git-send-email-hejianet@gmail.com>
From: Jia He <hejianet@gmail.com>
Date: Wed, 28 Sep 2016 14:22:21 +0800
> v5:
> - order local variables from longest to shortest line
I still see many cases where this problem still exists. Please
do not resubmit this patch series until you fix all of them.
Patch #2:
-static int snmp_seq_show(struct seq_file *seq, void *v)
+static int snmp_seq_show_ipstats(struct seq_file *seq, void *v)
{
int i;
+ u64 buff64[IPSTATS_MIB_MAX];
struct net *net = seq->private;
The order should be "net" then "buff64" then "i".
+static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
+{
+ int i;
+ struct net *net = seq->private;
+ unsigned long buff[TCPUDP_MIB_MAX];
The order should be "buff", "net", then "i".
Patch #3:
@@ -192,13 +197,19 @@ static void snmp6_seq_show_item(struct seq_file *seq, void __percpu *pcpumib,
const struct snmp_mib *itemlist)
{
int i;
- unsigned long val;
-
...
+ unsigned long buff[SNMP_MIB_MAX];
The order should be "buff" then "i".
@@ -206,10 +217,13 @@ static void snmp6_seq_show_item64(struct seq_file *seq, void __percpu *mib,
const struct snmp_mib *itemlist, size_t syncpoff)
{
int i;
+ u64 buff64[SNMP_MIB_MAX];
Likewise.
I cannot be any more explicit in my request than this.
^ permalink raw reply
* Re: [PATCH net-next V2] net/sched: pkt_cls: change tc actions order to be as the user sets
From: David Miller @ 2016-09-28 9:03 UTC (permalink / raw)
To: hadarh; +Cc: netdev, jhs, xiyou.wangcong, ogerlitz
In-Reply-To: <1474963791-3717-1-git-send-email-hadarh@mellanox.com>
From: Hadar Hen Zion <hadarh@mellanox.com>
Date: Tue, 27 Sep 2016 11:09:51 +0300
> Currently the created tc actions list is reversed against the order
> set by the user.
> Change the actions list order to be the same as was set by the user.
>
> This patch doesn't affect dump actions behavior.
> For dumping, action->order parameter is used so the list order doesn't
> matter.
>
> Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Applied, thanks.
^ permalink raw reply
* [PATCH v5] net: ip, diag -- Add diag interface for raw sockets
From: Cyrill Gorcunov @ 2016-09-28 9:03 UTC (permalink / raw)
To: Eric Dumazet, David Ahern
Cc: netdev, linux-kernel, David Miller, kuznet, jmorris, yoshfuji,
kaber, avagin, stephen
In criu we are actively using diag interface to collect sockets
present in the system when dumping applications. And while for
unix, tcp, udp[lite], packet, netlink it works as expected,
the raw sockets do not have. Thus add it.
v2:
- add missing sock_put calls in raw_diag_dump_one (by eric.dumazet@)
- implement @destroy for diag requests (by dsa@)
v3:
- add export of raw_abort for IPv6 (by dsa@)
- pass net-admin flag into inet_sk_diag_fill due to
changes in net-next branch (by dsa@)
v4:
- use @pad in struct inet_diag_req_v2 for raw socket
protocol specification: raw module carries sockets
which may have custom protocol passed from socket()
syscall and sole @sdiag_protocol is not enough to
match underlied ones
- start reporting protocol specifed in socket() call
when sockets are raw ones for the same reason: user
space tools like ss may parse this attribute and use
it for socket matching
v5 (by eric.dumazet@):
- use sock_hold in raw_sock_get instead of atomic_inc,
we're holding (raw_v4_hashinfo|raw_v6_hashinfo)->lock
when looking up so counter won't be zero here.
CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <eric.dumazet@gmail.com>
CC: David Ahern <dsa@cumulusnetworks.com>
CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
CC: James Morris <jmorris@namei.org>
CC: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
CC: Patrick McHardy <kaber@trash.net>
CC: Andrey Vagin <avagin@openvz.org>
CC: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
Thanks all for feedback! Take a look please once time permit.
include/net/raw.h | 6 +
include/net/rawv6.h | 7 +
include/uapi/linux/inet_diag.h | 5
net/ipv4/Kconfig | 8 +
net/ipv4/Makefile | 1
net/ipv4/inet_diag.c | 9 +
net/ipv4/raw.c | 21 +++
net/ipv4/raw_diag.c | 233 +++++++++++++++++++++++++++++++++++++++++
net/ipv6/raw.c | 7 -
9 files changed, 292 insertions(+), 5 deletions(-)
Index: linux-ml.git/include/net/raw.h
===================================================================
--- linux-ml.git.orig/include/net/raw.h
+++ linux-ml.git/include/net/raw.h
@@ -23,6 +23,12 @@
extern struct proto raw_prot;
+extern struct raw_hashinfo raw_v4_hashinfo;
+struct sock *__raw_v4_lookup(struct net *net, struct sock *sk,
+ unsigned short num, __be32 raddr,
+ __be32 laddr, int dif);
+
+int raw_abort(struct sock *sk, int err);
void raw_icmp_error(struct sk_buff *, int, u32);
int raw_local_deliver(struct sk_buff *, int);
Index: linux-ml.git/include/net/rawv6.h
===================================================================
--- linux-ml.git.orig/include/net/rawv6.h
+++ linux-ml.git/include/net/rawv6.h
@@ -3,6 +3,13 @@
#include <net/protocol.h>
+extern struct raw_hashinfo raw_v6_hashinfo;
+struct sock *__raw_v6_lookup(struct net *net, struct sock *sk,
+ unsigned short num, const struct in6_addr *loc_addr,
+ const struct in6_addr *rmt_addr, int dif);
+
+int raw_abort(struct sock *sk, int err);
+
void raw6_icmp_error(struct sk_buff *, int nexthdr,
u8 type, u8 code, int inner_offset, __be32);
bool raw6_local_deliver(struct sk_buff *, int);
Index: linux-ml.git/include/uapi/linux/inet_diag.h
===================================================================
--- linux-ml.git.orig/include/uapi/linux/inet_diag.h
+++ linux-ml.git/include/uapi/linux/inet_diag.h
@@ -38,7 +38,10 @@ struct inet_diag_req_v2 {
__u8 sdiag_family;
__u8 sdiag_protocol;
__u8 idiag_ext;
- __u8 pad;
+ union {
+ __u8 pad;
+ __u8 sdiag_raw_protocol; /* SOCK_RAW only, @pad for others */
+ };
__u32 idiag_states;
struct inet_diag_sockid id;
};
Index: linux-ml.git/net/ipv4/Kconfig
===================================================================
--- linux-ml.git.orig/net/ipv4/Kconfig
+++ linux-ml.git/net/ipv4/Kconfig
@@ -430,6 +430,14 @@ config INET_UDP_DIAG
Support for UDP socket monitoring interface used by the ss tool.
If unsure, say Y.
+config INET_RAW_DIAG
+ tristate "RAW: socket monitoring interface"
+ depends on INET_DIAG && (IPV6 || IPV6=n)
+ default n
+ ---help---
+ Support for RAW socket monitoring interface used by the ss tool.
+ If unsure, say Y.
+
config INET_DIAG_DESTROY
bool "INET: allow privileged process to administratively close sockets"
depends on INET_DIAG
Index: linux-ml.git/net/ipv4/Makefile
===================================================================
--- linux-ml.git.orig/net/ipv4/Makefile
+++ linux-ml.git/net/ipv4/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_NETFILTER) += netfilter.o n
obj-$(CONFIG_INET_DIAG) += inet_diag.o
obj-$(CONFIG_INET_TCP_DIAG) += tcp_diag.o
obj-$(CONFIG_INET_UDP_DIAG) += udp_diag.o
+obj-$(CONFIG_INET_RAW_DIAG) += raw_diag.o
obj-$(CONFIG_NET_TCPPROBE) += tcp_probe.o
obj-$(CONFIG_TCP_CONG_BIC) += tcp_bic.o
obj-$(CONFIG_TCP_CONG_CDG) += tcp_cdg.o
Index: linux-ml.git/net/ipv4/inet_diag.c
===================================================================
--- linux-ml.git.orig/net/ipv4/inet_diag.c
+++ linux-ml.git/net/ipv4/inet_diag.c
@@ -200,6 +200,15 @@ int inet_sk_diag_fill(struct sock *sk, s
if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
goto errout;
+ /*
+ * RAW sockets might have user-defined protocols assigned,
+ * so report the one supplied on socket creation.
+ */
+ if (sk->sk_type == SOCK_RAW) {
+ if (nla_put_u8(skb, INET_DIAG_PROTOCOL, sk->sk_protocol))
+ goto errout;
+ }
+
if (!icsk) {
handler->idiag_get_info(sk, r, NULL);
goto out;
Index: linux-ml.git/net/ipv4/raw.c
===================================================================
--- linux-ml.git.orig/net/ipv4/raw.c
+++ linux-ml.git/net/ipv4/raw.c
@@ -89,9 +89,10 @@ struct raw_frag_vec {
int hlen;
};
-static struct raw_hashinfo raw_v4_hashinfo = {
+struct raw_hashinfo raw_v4_hashinfo = {
.lock = __RW_LOCK_UNLOCKED(raw_v4_hashinfo.lock),
};
+EXPORT_SYMBOL_GPL(raw_v4_hashinfo);
int raw_hash_sk(struct sock *sk)
{
@@ -120,7 +121,7 @@ void raw_unhash_sk(struct sock *sk)
}
EXPORT_SYMBOL_GPL(raw_unhash_sk);
-static struct sock *__raw_v4_lookup(struct net *net, struct sock *sk,
+struct sock *__raw_v4_lookup(struct net *net, struct sock *sk,
unsigned short num, __be32 raddr, __be32 laddr, int dif)
{
sk_for_each_from(sk) {
@@ -136,6 +137,7 @@ static struct sock *__raw_v4_lookup(stru
found:
return sk;
}
+EXPORT_SYMBOL_GPL(__raw_v4_lookup);
/*
* 0 - deliver
@@ -918,6 +920,20 @@ static int compat_raw_ioctl(struct sock
}
#endif
+int raw_abort(struct sock *sk, int err)
+{
+ lock_sock(sk);
+
+ sk->sk_err = err;
+ sk->sk_error_report(sk);
+ udp_disconnect(sk, 0);
+
+ release_sock(sk);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(raw_abort);
+
struct proto raw_prot = {
.name = "RAW",
.owner = THIS_MODULE,
@@ -943,6 +959,7 @@ struct proto raw_prot = {
.compat_getsockopt = compat_raw_getsockopt,
.compat_ioctl = compat_raw_ioctl,
#endif
+ .diag_destroy = raw_abort,
};
#ifdef CONFIG_PROC_FS
Index: linux-ml.git/net/ipv4/raw_diag.c
===================================================================
--- /dev/null
+++ linux-ml.git/net/ipv4/raw_diag.c
@@ -0,0 +1,233 @@
+#include <linux/module.h>
+
+#include <linux/inet_diag.h>
+#include <linux/sock_diag.h>
+
+#include <net/raw.h>
+#include <net/rawv6.h>
+
+#ifdef pr_fmt
+# undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+static struct raw_hashinfo *
+raw_get_hashinfo(const struct inet_diag_req_v2 *r)
+{
+ if (r->sdiag_family == AF_INET) {
+ return &raw_v4_hashinfo;
+#if IS_ENABLED(CONFIG_IPV6)
+ } else if (r->sdiag_family == AF_INET6) {
+ return &raw_v6_hashinfo;
+#endif
+ } else {
+ pr_warn_once("Unexpected inet family %d\n",
+ r->sdiag_family);
+ WARN_ON_ONCE(1);
+ return ERR_PTR(-EINVAL);
+ }
+}
+
+static struct sock *raw_lookup(struct net *net, struct sock *from,
+ const struct inet_diag_req_v2 *r)
+{
+ struct sock *sk = NULL;
+
+ if (r->sdiag_family == AF_INET)
+ sk = __raw_v4_lookup(net, from, r->sdiag_raw_protocol,
+ r->id.idiag_dst[0],
+ r->id.idiag_src[0],
+ r->id.idiag_if);
+#if IS_ENABLED(CONFIG_IPV6)
+ else
+ sk = __raw_v6_lookup(net, from, r->sdiag_raw_protocol,
+ (const struct in6_addr *)r->id.idiag_src,
+ (const struct in6_addr *)r->id.idiag_dst,
+ r->id.idiag_if);
+#endif
+ return sk;
+}
+
+static struct sock *raw_sock_get(struct net *net, const struct inet_diag_req_v2 *r)
+{
+ struct raw_hashinfo *hashinfo = raw_get_hashinfo(r);
+ struct sock *sk = NULL, *s;
+ int slot;
+
+ if (IS_ERR(hashinfo))
+ return ERR_CAST(hashinfo);
+
+ read_lock(&hashinfo->lock);
+ for (slot = 0; slot < RAW_HTABLE_SIZE; slot++) {
+ sk_for_each(s, &hashinfo->ht[slot]) {
+ sk = raw_lookup(net, s, r);
+ if (sk) {
+ /*
+ * Grab it and keep until we fill
+ * diag message to be reported, so
+ * caller should call sock_put then.
+ * We can do that because we're keeping
+ * hashinfo->lock here.
+ */
+ sock_hold(sk);
+ break;
+ }
+ }
+ }
+ read_unlock(&hashinfo->lock);
+
+ return sk ? sk : ERR_PTR(-ENOENT);
+}
+
+static int raw_diag_dump_one(struct sk_buff *in_skb,
+ const struct nlmsghdr *nlh,
+ const struct inet_diag_req_v2 *r)
+{
+ struct net *net = sock_net(in_skb->sk);
+ struct sk_buff *rep;
+ struct sock *sk;
+ int err;
+
+ sk = raw_sock_get(net, r);
+ if (IS_ERR(sk))
+ return PTR_ERR(sk);
+
+ rep = nlmsg_new(sizeof(struct inet_diag_msg) +
+ sizeof(struct inet_diag_meminfo) + 64,
+ GFP_KERNEL);
+ if (!rep) {
+ sock_put(sk);
+ return -ENOMEM;
+ }
+
+ err = inet_sk_diag_fill(sk, NULL, rep, r,
+ sk_user_ns(NETLINK_CB(in_skb).sk),
+ NETLINK_CB(in_skb).portid,
+ nlh->nlmsg_seq, 0, nlh,
+ netlink_net_capable(in_skb, CAP_NET_ADMIN));
+ sock_put(sk);
+
+ if (err < 0) {
+ kfree_skb(rep);
+ return err;
+ }
+
+ err = netlink_unicast(net->diag_nlsk, rep,
+ NETLINK_CB(in_skb).portid,
+ MSG_DONTWAIT);
+ if (err > 0)
+ err = 0;
+ return err;
+}
+
+static int sk_diag_dump(struct sock *sk, struct sk_buff *skb,
+ struct netlink_callback *cb,
+ const struct inet_diag_req_v2 *r,
+ struct nlattr *bc, bool net_admin)
+{
+ if (!inet_diag_bc_sk(bc, sk))
+ return 0;
+
+ return inet_sk_diag_fill(sk, NULL, skb, r,
+ sk_user_ns(NETLINK_CB(cb->skb).sk),
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq, NLM_F_MULTI,
+ cb->nlh, net_admin);
+}
+
+static void raw_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
+ const struct inet_diag_req_v2 *r, struct nlattr *bc)
+{
+ bool net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN);
+ struct raw_hashinfo *hashinfo = raw_get_hashinfo(r);
+ struct net *net = sock_net(skb->sk);
+ int num, s_num, slot, s_slot;
+ struct sock *sk = NULL;
+
+ if (IS_ERR(hashinfo))
+ return;
+
+ s_slot = cb->args[0];
+ num = s_num = cb->args[1];
+
+ read_lock(&hashinfo->lock);
+ for (slot = s_slot; slot < RAW_HTABLE_SIZE; s_num = 0, slot++) {
+ num = 0;
+
+ sk_for_each(sk, &hashinfo->ht[slot]) {
+ struct inet_sock *inet = inet_sk(sk);
+
+ if (!net_eq(sock_net(sk), net))
+ continue;
+ if (num < s_num)
+ goto next;
+ if (sk->sk_family != r->sdiag_family)
+ goto next;
+ if (r->id.idiag_sport != inet->inet_sport &&
+ r->id.idiag_sport)
+ goto next;
+ if (r->id.idiag_dport != inet->inet_dport &&
+ r->id.idiag_dport)
+ goto next;
+ if (sk_diag_dump(sk, skb, cb, r, bc, net_admin) < 0)
+ goto out_unlock;
+next:
+ num++;
+ }
+ }
+
+out_unlock:
+ read_unlock(&hashinfo->lock);
+
+ cb->args[0] = slot;
+ cb->args[1] = num;
+}
+
+static void raw_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
+ void *info)
+{
+ r->idiag_rqueue = sk_rmem_alloc_get(sk);
+ r->idiag_wqueue = sk_wmem_alloc_get(sk);
+}
+
+#ifdef CONFIG_INET_DIAG_DESTROY
+static int raw_diag_destroy(struct sk_buff *in_skb,
+ const struct inet_diag_req_v2 *r)
+{
+ struct net *net = sock_net(in_skb->sk);
+ struct sock *sk;
+
+ sk = raw_sock_get(net, r);
+ if (IS_ERR(sk))
+ return PTR_ERR(sk);
+ return sock_diag_destroy(sk, ECONNABORTED);
+}
+#endif
+
+static const struct inet_diag_handler raw_diag_handler = {
+ .dump = raw_diag_dump,
+ .dump_one = raw_diag_dump_one,
+ .idiag_get_info = raw_diag_get_info,
+ .idiag_type = IPPROTO_RAW,
+ .idiag_info_size = 0,
+#ifdef CONFIG_INET_DIAG_DESTROY
+ .destroy = raw_diag_destroy,
+#endif
+};
+
+static int __init raw_diag_init(void)
+{
+ return inet_diag_register(&raw_diag_handler);
+}
+
+static void __exit raw_diag_exit(void)
+{
+ inet_diag_unregister(&raw_diag_handler);
+}
+
+module_init(raw_diag_init);
+module_exit(raw_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, 2-255 /* AF_INET - IPPROTO_RAW */);
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, 10-255 /* AF_INET6 - IPPROTO_RAW */);
Index: linux-ml.git/net/ipv6/raw.c
===================================================================
--- linux-ml.git.orig/net/ipv6/raw.c
+++ linux-ml.git/net/ipv6/raw.c
@@ -65,11 +65,12 @@
#define ICMPV6_HDRLEN 4 /* ICMPv6 header, RFC 4443 Section 2.1 */
-static struct raw_hashinfo raw_v6_hashinfo = {
+struct raw_hashinfo raw_v6_hashinfo = {
.lock = __RW_LOCK_UNLOCKED(raw_v6_hashinfo.lock),
};
+EXPORT_SYMBOL_GPL(raw_v6_hashinfo);
-static struct sock *__raw_v6_lookup(struct net *net, struct sock *sk,
+struct sock *__raw_v6_lookup(struct net *net, struct sock *sk,
unsigned short num, const struct in6_addr *loc_addr,
const struct in6_addr *rmt_addr, int dif)
{
@@ -102,6 +103,7 @@ static struct sock *__raw_v6_lookup(stru
found:
return sk;
}
+EXPORT_SYMBOL_GPL(__raw_v6_lookup);
/*
* 0 - deliver
@@ -1252,6 +1254,7 @@ struct proto rawv6_prot = {
.compat_getsockopt = compat_rawv6_getsockopt,
.compat_ioctl = compat_rawv6_ioctl,
#endif
+ .diag_destroy = raw_abort,
};
#ifdef CONFIG_PROC_FS
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox