netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
@ 2015-03-02 13:14 Eyal Birger
  2015-03-02 13:14 ` [PATCH net-next v4 1/2] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv() Eyal Birger
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 13:14 UTC (permalink / raw)
  To: davem; +Cc: willemb, edumazet, shmulik.ladkani, netdev, Eyal Birger

This patch set introduces a new socket option for fetching the mark
of skbs passed to sockets as ancillary data.

A userspace program may wish to receive the mark of packets it
receives, for example for distinguishing between different TPROXY
diversion rules to the same userspace proxy socket.

The patch set includes a minor function renaming.

---
Changes in v4:
- Rebase
- skb->mark is no longer aliased, so skb->priority is left as it is
  and change in IGMPv3/MLD is no longer needed
- rxrpc no longer uses sock_recv_ts_and_drops() so renaming there is
  not needed
Changes in v3:
- Rebase
- Set skb->priority immediately prior to transmittion in IGMPv3/MLD
- Extended commit message regarding use of skb->priority instead of
  skb->mark in struct sk_buff union
Changes in v2:
- Function parameter indentation
- Commit message formatting
---

Eyal Birger (2):
  net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv()
  net: Introducing socket mark receive socket option

 arch/alpha/include/uapi/asm/socket.h   |  2 ++
 arch/avr32/include/uapi/asm/socket.h   |  2 ++
 arch/cris/include/uapi/asm/socket.h    |  2 ++
 arch/frv/include/uapi/asm/socket.h     |  2 ++
 arch/ia64/include/uapi/asm/socket.h    |  2 ++
 arch/m32r/include/uapi/asm/socket.h    |  2 ++
 arch/mips/include/uapi/asm/socket.h    |  2 ++
 arch/mn10300/include/uapi/asm/socket.h |  2 ++
 arch/parisc/include/uapi/asm/socket.h  |  2 ++
 arch/powerpc/include/uapi/asm/socket.h |  2 ++
 arch/s390/include/uapi/asm/socket.h    |  2 ++
 arch/sparc/include/uapi/asm/socket.h   |  2 ++
 arch/xtensa/include/uapi/asm/socket.h  |  2 ++
 include/net/sock.h                     | 18 ++++++++++--------
 include/uapi/asm-generic/socket.h      |  2 ++
 net/atm/common.c                       |  2 +-
 net/bluetooth/af_bluetooth.c           |  4 ++--
 net/can/bcm.c                          |  2 +-
 net/can/raw.c                          |  2 +-
 net/core/sock.c                        |  8 ++++++++
 net/ieee802154/socket.c                |  4 ++--
 net/ipv4/raw.c                         |  2 +-
 net/ipv4/udp.c                         |  2 +-
 net/ipv6/raw.c                         |  2 +-
 net/ipv6/udp.c                         |  2 +-
 net/key/af_key.c                       |  2 +-
 net/packet/af_packet.c                 |  2 +-
 net/sctp/socket.c                      |  2 +-
 net/socket.c                           | 15 ++++++++++++---
 29 files changed, 72 insertions(+), 25 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 1/2] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv()
  2015-03-02 13:14 [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option Eyal Birger
@ 2015-03-02 13:14 ` Eyal Birger
  2015-03-02 13:14 ` [PATCH net-next v4 2/2] net: Introducing socket mark receive socket option Eyal Birger
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 13:14 UTC (permalink / raw)
  To: davem; +Cc: willemb, edumazet, shmulik.ladkani, netdev, Eyal Birger

sock_recv_ts_and_drops() - as its name suggests - allows receiving
timestamp information and drop statistics.
Generalize the function name in preparation for adding additional
ancillary information.

Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
 include/net/sock.h           | 10 +++++-----
 net/atm/common.c             |  2 +-
 net/bluetooth/af_bluetooth.c |  4 ++--
 net/can/bcm.c                |  2 +-
 net/can/raw.c                |  2 +-
 net/ieee802154/socket.c      |  4 ++--
 net/ipv4/raw.c               |  2 +-
 net/ipv4/udp.c               |  2 +-
 net/ipv6/raw.c               |  2 +-
 net/ipv6/udp.c               |  2 +-
 net/key/af_key.c             |  2 +-
 net/packet/af_packet.c       |  2 +-
 net/sctp/socket.c            |  2 +-
 net/socket.c                 |  6 +++---
 14 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 38369d3..56d5980 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2131,11 +2131,11 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
 		__sock_recv_wifi_status(msg, sk, skb);
 }
 
-void __sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
-			      struct sk_buff *skb);
+void __sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
+		      struct sk_buff *skb);
 
-static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
-					  struct sk_buff *skb)
+static inline void sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
+				  struct sk_buff *skb)
 {
 #define FLAGS_TS_OR_DROPS ((1UL << SOCK_RXQ_OVFL)			| \
 			   (1UL << SOCK_RCVTSTAMP))
@@ -2143,7 +2143,7 @@ static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
 			   SOF_TIMESTAMPING_RAW_HARDWARE)
 
 	if (sk->sk_flags & FLAGS_TS_OR_DROPS || sk->sk_tsflags & TSFLAGS_ANY)
-		__sock_recv_ts_and_drops(msg, sk, skb);
+		__sock_cmsg_recv(msg, sk, skb);
 	else
 		sk->sk_stamp = skb->tstamp;
 }
diff --git a/net/atm/common.c b/net/atm/common.c
index b84057e..611b1c2 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -557,7 +557,7 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 	error = skb_copy_datagram_msg(skb, 0, msg, copied);
 	if (error)
 		return error;
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (!(flags & MSG_PEEK)) {
 		pr_debug("%d -= %d\n", atomic_read(&sk->sk_rmem_alloc),
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 4b904c9..56aeaa3 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -241,7 +241,7 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 	skb_reset_transport_header(skb);
 	err = skb_copy_datagram_msg(skb, 0, msg, copied);
 	if (err == 0) {
-		sock_recv_ts_and_drops(msg, sk, skb);
+		sock_cmsg_recv(msg, sk, skb);
 
 		if (bt_sk(sk)->skb_msg_name)
 			bt_sk(sk)->skb_msg_name(skb, msg->msg_name,
@@ -339,7 +339,7 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
 		copied += chunk;
 		size   -= chunk;
 
-		sock_recv_ts_and_drops(msg, sk, skb);
+		sock_cmsg_recv(msg, sk, skb);
 
 		if (!(flags & MSG_PEEK)) {
 			int skb_len = skb_headlen(skb);
diff --git a/net/can/bcm.c b/net/can/bcm.c
index d559f92..3de5ed1 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1559,7 +1559,7 @@ static int bcm_recvmsg(struct kiocb *iocb, struct socket *sock,
 		return err;
 	}
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (msg->msg_name) {
 		__sockaddr_check_size(sizeof(struct sockaddr_can));
diff --git a/net/can/raw.c b/net/can/raw.c
index 94601b7..c5988a4 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -754,7 +754,7 @@ static int raw_recvmsg(struct kiocb *iocb, struct socket *sock,
 		return err;
 	}
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (msg->msg_name) {
 		__sockaddr_check_size(sizeof(struct sockaddr_can));
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 2878d8c..41ac5fa 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -348,7 +348,7 @@ static int raw_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	if (err)
 		goto done;
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (flags & MSG_TRUNC)
 		copied = skb->len;
@@ -739,7 +739,7 @@ static int dgram_recvmsg(struct kiocb *iocb, struct sock *sk,
 	if (err)
 		goto done;
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (saddr) {
 		saddr->family = AF_IEEE802154;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index f027a70..a28a83b 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -740,7 +740,7 @@ static int raw_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	if (err)
 		goto done;
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	/* Copy the address. */
 	if (sin) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0224f93..22591b9 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1318,7 +1318,7 @@ try_again:
 		UDP_INC_STATS_USER(sock_net(sk),
 				UDP_MIB_INDATAGRAMS, is_udplite);
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	/* Copy the address. */
 	if (sin) {
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 0d84b2c..e810faa 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -510,7 +510,7 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 		*addr_len = sizeof(*sin6);
 	}
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (np->rxopt.all)
 		ip6_datagram_recv_ctl(sk, msg, skb);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index d048d46..be0cfae 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -469,7 +469,7 @@ try_again:
 					UDP_MIB_INDATAGRAMS, is_udplite);
 	}
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	/* Copy the address. */
 	if (msg->msg_name) {
diff --git a/net/key/af_key.c b/net/key/af_key.c
index f8ac939..e03ba49 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3658,7 +3658,7 @@ static int pfkey_recvmsg(struct kiocb *kiocb,
 	if (err)
 		goto out_free;
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	err = (flags & MSG_TRUNC) ? skb->len : copied;
 
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 9db8369..4e2f54c 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2998,7 +2998,7 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
 		sll->sll_protocol = skb->protocol;
 	}
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 
 	if (msg->msg_name) {
 		/* If the address length field is there to be filled
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index aafe94b..27835aa 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2108,7 +2108,7 @@ static int sctp_recvmsg(struct kiocb *iocb, struct sock *sk,
 	if (err)
 		goto out_free;
 
-	sock_recv_ts_and_drops(msg, sk, skb);
+	sock_cmsg_recv(msg, sk, skb);
 	if (sctp_ulpevent_is_notification(event)) {
 		msg->msg_flags |= MSG_NOTIFICATION;
 		sp->pf->event_msgname(event, msg->msg_name, addr_len);
diff --git a/net/socket.c b/net/socket.c
index b78cf60..b75eb3f 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -736,13 +736,13 @@ static inline void sock_recv_drops(struct msghdr *msg, struct sock *sk,
 			sizeof(__u32), &SOCK_SKB_CB(skb)->dropcount);
 }
 
-void __sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
-	struct sk_buff *skb)
+void __sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
+		      struct sk_buff *skb)
 {
 	sock_recv_timestamp(msg, sk, skb);
 	sock_recv_drops(msg, sk, skb);
 }
-EXPORT_SYMBOL_GPL(__sock_recv_ts_and_drops);
+EXPORT_SYMBOL_GPL(__sock_cmsg_recv);
 
 static inline int __sock_recvmsg_nosec(struct kiocb *iocb, struct socket *sock,
 				       struct msghdr *msg, size_t size, int flags)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 2/2] net: Introducing socket mark receive socket option
  2015-03-02 13:14 [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option Eyal Birger
  2015-03-02 13:14 ` [PATCH net-next v4 1/2] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv() Eyal Birger
@ 2015-03-02 13:14 ` Eyal Birger
  2015-03-02 13:29 ` [PATCH net-next v4 0/2] " Florian Westphal
  2015-03-02 20:01 ` David Miller
  3 siblings, 0 replies; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 13:14 UTC (permalink / raw)
  To: davem; +Cc: willemb, edumazet, shmulik.ladkani, netdev, Eyal Birger

A userspace program may wish to receive the mark of packets it
receives.

Packets may be marked by Netfilter, by other userspace applications
using the SO_MARK socket option, or by other kernel means.

Receiving the mark in userspace is useful for example for
distinguishing between different TPROXY diversion rules to the same
userspace proxy socket.

Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
 arch/alpha/include/uapi/asm/socket.h   | 2 ++
 arch/avr32/include/uapi/asm/socket.h   | 2 ++
 arch/cris/include/uapi/asm/socket.h    | 2 ++
 arch/frv/include/uapi/asm/socket.h     | 2 ++
 arch/ia64/include/uapi/asm/socket.h    | 2 ++
 arch/m32r/include/uapi/asm/socket.h    | 2 ++
 arch/mips/include/uapi/asm/socket.h    | 2 ++
 arch/mn10300/include/uapi/asm/socket.h | 2 ++
 arch/parisc/include/uapi/asm/socket.h  | 2 ++
 arch/powerpc/include/uapi/asm/socket.h | 2 ++
 arch/s390/include/uapi/asm/socket.h    | 2 ++
 arch/sparc/include/uapi/asm/socket.h   | 2 ++
 arch/xtensa/include/uapi/asm/socket.h  | 2 ++
 include/net/sock.h                     | 8 +++++---
 include/uapi/asm-generic/socket.h      | 2 ++
 net/core/sock.c                        | 8 ++++++++
 net/socket.c                           | 9 +++++++++
 17 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 9a20821..7c49f4b 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -92,4 +92,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/avr32/include/uapi/asm/socket.h b/arch/avr32/include/uapi/asm/socket.h
index 2b65ed6..6c81e0e 100644
--- a/arch/avr32/include/uapi/asm/socket.h
+++ b/arch/avr32/include/uapi/asm/socket.h
@@ -85,4 +85,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _UAPI__ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/uapi/asm/socket.h b/arch/cris/include/uapi/asm/socket.h
index e2503d9f..55ca3be 100644
--- a/arch/cris/include/uapi/asm/socket.h
+++ b/arch/cris/include/uapi/asm/socket.h
@@ -87,6 +87,8 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_SOCKET_H */
 
 
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index 4823ad1..b69f0d5 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -85,5 +85,7 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 59be3d8..ee6abaf 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -94,4 +94,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index 7bc4cb2..bf0f932 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -85,4 +85,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index dec3c85..47a61c4 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -103,4 +103,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index cab7d6d..216305b 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -85,4 +85,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index a5cd40c..047980a3 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -84,4 +84,6 @@
 #define SO_ATTACH_BPF		0x402B
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		0x402C
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index c046666..188abb6 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -92,4 +92,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index 296942d..70a5791 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -91,4 +91,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index e6a16c4..f7ee249 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -81,6 +81,8 @@
 #define SO_ATTACH_BPF		0x0034
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		0x0035
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index 4120af0..5b588fa 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -96,4 +96,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 56d5980..48318cc 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -719,6 +719,7 @@ enum sock_flags {
 		     */
 	SOCK_FILTER_LOCKED, /* Filter cannot be changed anymore */
 	SOCK_SELECT_ERR_QUEUE, /* Wake select on error queue */
+	SOCK_RCVMARK,
 };
 
 static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)
@@ -2137,12 +2138,13 @@ void __sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
 static inline void sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
 				  struct sk_buff *skb)
 {
-#define FLAGS_TS_OR_DROPS ((1UL << SOCK_RXQ_OVFL)			| \
-			   (1UL << SOCK_RCVTSTAMP))
+#define FLAGS_CMSG_ANY	  ((1UL << SOCK_RXQ_OVFL)			| \
+			   (1UL << SOCK_RCVTSTAMP)			| \
+			   (1UL << SOCK_RCVMARK))
 #define TSFLAGS_ANY	  (SOF_TIMESTAMPING_SOFTWARE			| \
 			   SOF_TIMESTAMPING_RAW_HARDWARE)
 
-	if (sk->sk_flags & FLAGS_TS_OR_DROPS || sk->sk_tsflags & TSFLAGS_ANY)
+	if (sk->sk_flags & FLAGS_CMSG_ANY || sk->sk_tsflags & TSFLAGS_ANY)
 		__sock_cmsg_recv(msg, sk, skb);
 	else
 		sk->sk_stamp = skb->tstamp;
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 5c15c2a..17a1e2e 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -87,4 +87,6 @@
 #define SO_ATTACH_BPF		50
 #define SO_DETACH_BPF		SO_DETACH_FILTER
 
+#define SO_RCVMARK		51
+
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index 9c74fc8..3f8ba81 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -928,6 +928,10 @@ set_rcvbuf:
 			sk->sk_mark = val;
 		break;
 
+	case SO_RCVMARK:
+		sock_valbool_flag(sk, SOCK_RCVMARK, valbool);
+		break;
+
 		/* We implement the SO_SNDLOWAT etc to
 		   not be settable (1003.1g 5.3) */
 	case SO_RXQ_OVFL:
@@ -1179,6 +1183,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_mark;
 		break;
 
+	case SO_RCVMARK:
+		v.val = sock_flag(sk, SOCK_RCVMARK);
+		break;
+
 	case SO_RXQ_OVFL:
 		v.val = sock_flag(sk, SOCK_RXQ_OVFL);
 		break;
diff --git a/net/socket.c b/net/socket.c
index b75eb3f..c1db5d5 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -736,11 +736,20 @@ static inline void sock_recv_drops(struct msghdr *msg, struct sock *sk,
 			sizeof(__u32), &SOCK_SKB_CB(skb)->dropcount);
 }
 
+static inline void sock_recv_mark(struct msghdr *msg, struct sock *sk,
+				  struct sk_buff *skb)
+{
+	if (sock_flag(sk, SOCK_RCVMARK) && skb)
+		put_cmsg(msg, SOL_SOCKET, SO_RCVMARK,
+			 sizeof(__u32), &skb->mark);
+}
+
 void __sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
 		      struct sk_buff *skb)
 {
 	sock_recv_timestamp(msg, sk, skb);
 	sock_recv_drops(msg, sk, skb);
+	sock_recv_mark(msg, sk, skb);
 }
 EXPORT_SYMBOL_GPL(__sock_cmsg_recv);
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 13:14 [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option Eyal Birger
  2015-03-02 13:14 ` [PATCH net-next v4 1/2] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv() Eyal Birger
  2015-03-02 13:14 ` [PATCH net-next v4 2/2] net: Introducing socket mark receive socket option Eyal Birger
@ 2015-03-02 13:29 ` Florian Westphal
  2015-03-02 13:48   ` Eyal Birger
  2015-03-02 20:01 ` David Miller
  3 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2015-03-02 13:29 UTC (permalink / raw)
  To: Eyal Birger; +Cc: davem, willemb, edumazet, shmulik.ladkani, netdev

Eyal Birger <eyal.birger@gmail.com> wrote:
> This patch set introduces a new socket option for fetching the mark
> of skbs passed to sockets as ancillary data.
> 
> A userspace program may wish to receive the mark of packets it
> receives, for example for distinguishing between different TPROXY
> diversion rules to the same userspace proxy socket.

Hmm... Whats the use case?
Even if you cannot use multiple sockets for every divert rule,
TPROXY doesn't mangle payload; applications could use sockaddrs
returned by accept, getpeername, getsockname etc.  to figure out
which original port/address the packet was sent to?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 13:29 ` [PATCH net-next v4 0/2] " Florian Westphal
@ 2015-03-02 13:48   ` Eyal Birger
  2015-03-02 14:36     ` Florian Westphal
  0 siblings, 1 reply; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 13:48 UTC (permalink / raw)
  To: Florian Westphal
  Cc: David Miller, Willem de Bruijn, Eric Dumazet, Shmulik Ladkani,
	netdev@vger.kernel.org

On Mon, Mar 2, 2015 at 3:29 PM, Florian Westphal <fw@strlen.de> wrote:
> Eyal Birger <eyal.birger@gmail.com> wrote:
>> This patch set introduces a new socket option for fetching the mark
>> of skbs passed to sockets as ancillary data.
>>
>> A userspace program may wish to receive the mark of packets it
>> receives, for example for distinguishing between different TPROXY
>> diversion rules to the same userspace proxy socket.
>
> Hmm... Whats the use case?
> Even if you cannot use multiple sockets for every divert rule,
> TPROXY doesn't mangle payload; applications could use sockaddrs
> returned by accept, getpeername, getsockname etc.  to figure out
> which original port/address the packet was sent to?

Right. But that would mean the criteria for traffic diversion would need to
be known to the application receiving the traffic. IMHO It is a more elegant
solution to use the mark in order to distinguish between different traffic flows
instead of duplicating the match logic.

Also, the feature has use-cases outside of TPROXY as the skb->mark may be set
by other mechanisms (including SO_MARK from user space).

For example, a user space daemon can receive traffic from multiple
applications using
a single socket and distinguish between different traffic groups
according to the packet
mark.

Regards,
Eyal.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 13:48   ` Eyal Birger
@ 2015-03-02 14:36     ` Florian Westphal
  2015-03-02 18:34       ` Eyal Birger
  2015-03-02 20:05       ` David Miller
  0 siblings, 2 replies; 16+ messages in thread
From: Florian Westphal @ 2015-03-02 14:36 UTC (permalink / raw)
  To: Eyal Birger
  Cc: Florian Westphal, David Miller, Willem de Bruijn, Eric Dumazet,
	Shmulik Ladkani, netdev@vger.kernel.org

Eyal Birger <eyal.birger@gmail.com> wrote:
> On Mon, Mar 2, 2015 at 3:29 PM, Florian Westphal <fw@strlen.de> wrote:
> > Eyal Birger <eyal.birger@gmail.com> wrote:
> >> This patch set introduces a new socket option for fetching the mark
> >> of skbs passed to sockets as ancillary data.
> >>
> >> A userspace program may wish to receive the mark of packets it
> >> receives, for example for distinguishing between different TPROXY
> >> diversion rules to the same userspace proxy socket.
> >
> > Hmm... Whats the use case?
> > Even if you cannot use multiple sockets for every divert rule,
> > TPROXY doesn't mangle payload; applications could use sockaddrs
> > returned by accept, getpeername, getsockname etc.  to figure out
> > which original port/address the packet was sent to?
> 
> Right. But that would mean the criteria for traffic diversion would need to
> be known to the application receiving the traffic.

For your solution to work the application needs to know about the TPROXY
rule set and how that is structured, no?

I don't see how that is 'better' than e.g. looking at dst port number.

> Also, the feature has use-cases outside of TPROXY as the skb->mark may be set
> by other mechanisms (including SO_MARK from user space).

Right, but to me it seems very hacky to use SO_MARK as some kind of OOB signal.

It won't work depending on loaded ruleset, it won't work with non-localhost
traffic and it won't work when other application runs in another network
namespace.

Seems such facility would be limited to some pre-configured distribution where
users don't run own software and make no changes to the default system
setup.

> For example, a user space daemon can receive traffic from multiple
> applications using a single socket and distinguish between different traffic groups
> according to the packet mark.

Right, but it might as well use SO_PEERCRED to identify the other pid, right?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 14:36     ` Florian Westphal
@ 2015-03-02 18:34       ` Eyal Birger
  2015-03-02 18:55         ` Florian Westphal
  2015-03-02 20:05       ` David Miller
  1 sibling, 1 reply; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 18:34 UTC (permalink / raw)
  To: Florian Westphal
  Cc: David Miller, Willem de Bruijn, Eric Dumazet, Shmulik Ladkani,
	netdev@vger.kernel.org

Hi,

On Mon, Mar 2, 2015 at 4:36 PM, Florian Westphal <fw@strlen.de> wrote:
> Eyal Birger <eyal.birger@gmail.com> wrote:
>> On Mon, Mar 2, 2015 at 3:29 PM, Florian Westphal <fw@strlen.de> wrote:
>> > Eyal Birger <eyal.birger@gmail.com> wrote:
>> >> This patch set introduces a new socket option for fetching the mark
>> >> of skbs passed to sockets as ancillary data.
>> >>
>> >> A userspace program may wish to receive the mark of packets it
>> >> receives, for example for distinguishing between different TPROXY
>> >> diversion rules to the same userspace proxy socket.
>> >
>> > Hmm... Whats the use case?
>> > Even if you cannot use multiple sockets for every divert rule,
>> > TPROXY doesn't mangle payload; applications could use sockaddrs
>> > returned by accept, getpeername, getsockname etc.  to figure out
>> > which original port/address the packet was sent to?
>>
>> Right. But that would mean the criteria for traffic diversion would need to
>> be known to the application receiving the traffic.
>
> For your solution to work the application needs to know about the TPROXY
> rule set and how that is structured, no?
>

The application does not need to know about the match criteria. Only about the
eventual mark. This decouples the semantics of a flow and it's actual
match criteria.

> I don't see how that is 'better' than e.g. looking at dst port number.

As mentioned, in cases where the match criteria is more complex than
just the dst
port number, the match logic has to be duplicated in the application.

>
>> Also, the feature has use-cases outside of TPROXY as the skb->mark may be set
>> by other mechanisms (including SO_MARK from user space).
>
> Right, but to me it seems very hacky to use SO_MARK as some kind of OOB signal.
>
> It won't work depending on loaded ruleset, it won't work with non-localhost
> traffic and it won't work when other application runs in another network
> namespace.
>
> Seems such facility would be limited to some pre-configured distribution where
> users don't run own software and make no changes to the default system
> setup.
>

It does not necessarily imply a static configuration, only a carefully
crafted one.
There are more than a few systems with this premise.

>> For example, a user space daemon can receive traffic from multiple
>> applications using a single socket and distinguish between different traffic groups
>> according to the packet mark.
>
> Right, but it might as well use SO_PEERCRED to identify the other pid, right?

I don't think so.

This would only work on connection/pair based sockets (and currently
only supported
in AF_UNIX) - the skb->mark can be different on a per packet basis -
especially when
several applications interact with a single daemon.

Regards,
Eyal.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 18:34       ` Eyal Birger
@ 2015-03-02 18:55         ` Florian Westphal
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2015-03-02 18:55 UTC (permalink / raw)
  To: Eyal Birger
  Cc: Florian Westphal, David Miller, Willem de Bruijn, Eric Dumazet,
	Shmulik Ladkani, netdev@vger.kernel.org

Eyal Birger <eyal.birger@gmail.com> wrote:
> On Mon, Mar 2, 2015 at 4:36 PM, Florian Westphal <fw@strlen.de> > > wrote:
> The application does not need to know about the match criteria. Only about the
> eventual mark. This decouples the semantics of a flow and it's actual
> match criteria.
> 
> > I don't see how that is 'better' than e.g. looking at dst port number.
> 
> As mentioned, in cases where the match criteria is more complex than
> just the dst
> port number, the match logic has to be duplicated in the application.

Sure.  However, in that case, I fail to see why you'd need to
differentiate at all; normally this would only be needed to e.g. figure
out if your proxy deals with f.e. http or ssl, and dport would be enough
for this.

> > Right, but to me it seems very hacky to use SO_MARK as some kind of OOB signal.
> >
> > It won't work depending on loaded ruleset, it won't work with non-localhost
> > traffic and it won't work when other application runs in another network
> > namespace.
> >
> > Seems such facility would be limited to some pre-configured distribution where
> > users don't run own software and make no changes to the default system
> > setup.
> >
> 
> It does not necessarily imply a static configuration, only a carefully
> crafted one.
> There are more than a few systems with this premise.

> >> For example, a user space daemon can receive traffic from multiple
> >> applications using a single socket and distinguish between different traffic groups
> >> according to the packet mark.
> >
> > Right, but it might as well use SO_PEERCRED to identify the other pid, right?
> 
> I don't think so.
> 
> This would only work on connection/pair based sockets (and currently
> only supported
> in AF_UNIX) - the skb->mark can be different on a per packet basis -
> especially when
> several applications interact with a single daemon.

Fair enough, I still  cannot imagine any scenario where doing this
would be a clean design or where this cannot be covered by other means
(in payload, via peercred, using dbus, etc.)

I'd have no problem at all with this if we had some kind of staging
tree for uapi :-}

I'll assume that I'm just not imaginative enough when it comes to use
cases for this facility, it doesn't seem to be too problematic exposing
this, aside from userspace considerations (such as inability to guarantee
that skb->mark will be set up as expected), so i still fail to see how
its useful for isolated applications or even a collection of programs
that want to do ipc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 13:14 [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option Eyal Birger
                   ` (2 preceding siblings ...)
  2015-03-02 13:29 ` [PATCH net-next v4 0/2] " Florian Westphal
@ 2015-03-02 20:01 ` David Miller
  3 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2015-03-02 20:01 UTC (permalink / raw)
  To: eyal.birger; +Cc: willemb, edumazet, shmulik.ladkani, netdev

From: Eyal Birger <eyal.birger@gmail.com>
Date: Mon,  2 Mar 2015 15:14:01 +0200

> This patch set introduces a new socket option for fetching the mark
> of skbs passed to sockets as ancillary data.
> 
> A userspace program may wish to receive the mark of packets it
> receives, for example for distinguishing between different TPROXY
> diversion rules to the same userspace proxy socket.
> 
> The patch set includes a minor function renaming.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 14:36     ` Florian Westphal
  2015-03-02 18:34       ` Eyal Birger
@ 2015-03-02 20:05       ` David Miller
  2015-03-02 20:38         ` Eyal Birger
  1 sibling, 1 reply; 16+ messages in thread
From: David Miller @ 2015-03-02 20:05 UTC (permalink / raw)
  To: fw; +Cc: eyal.birger, willemb, edumazet, shmulik.ladkani, netdev

From: Florian Westphal <fw@strlen.de>
Date: Mon, 2 Mar 2015 15:36:47 +0100

> Eyal Birger <eyal.birger@gmail.com> wrote:
>> On Mon, Mar 2, 2015 at 3:29 PM, Florian Westphal <fw@strlen.de> wrote:
>> > Eyal Birger <eyal.birger@gmail.com> wrote:
>> >> This patch set introduces a new socket option for fetching the mark
>> >> of skbs passed to sockets as ancillary data.
>> >>
>> >> A userspace program may wish to receive the mark of packets it
>> >> receives, for example for distinguishing between different TPROXY
>> >> diversion rules to the same userspace proxy socket.
>> >
>> > Hmm... Whats the use case?
>> > Even if you cannot use multiple sockets for every divert rule,
>> > TPROXY doesn't mangle payload; applications could use sockaddrs
>> > returned by accept, getpeername, getsockname etc.  to figure out
>> > which original port/address the packet was sent to?
>> 
>> Right. But that would mean the criteria for traffic diversion would need to
>> be known to the application receiving the traffic.
> 
> For your solution to work the application needs to know about the TPROXY
> rule set and how that is structured, no?
> 
> I don't see how that is 'better' than e.g. looking at dst port number.
 ...
>> For example, a user space daemon can receive traffic from multiple
>> applications using a single socket and distinguish between different traffic groups
>> according to the packet mark.
> 
> Right, but it might as well use SO_PEERCRED to identify the other pid, right?

Also, this points out that this socket option if accepted probably needs to
be privileged.

I can see administrators not being happy with applications being able to see
the marks used by their rulesets.

I'm backing out this series, Florian makes a lot of good points.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 20:05       ` David Miller
@ 2015-03-02 20:38         ` Eyal Birger
  2015-03-02 20:57           ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 20:38 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Westphal, Willem de Bruijn, Eric Dumazet, Shmulik Ladkani,
	netdev@vger.kernel.org

On Mon, Mar 2, 2015 at 10:05 PM, David Miller <davem@davemloft.net> wrote:
> From: Florian Westphal <fw@strlen.de>
> Date: Mon, 2 Mar 2015 15:36:47 +0100
>
>> Eyal Birger <eyal.birger@gmail.com> wrote:
>>> On Mon, Mar 2, 2015 at 3:29 PM, Florian Westphal <fw@strlen.de> wrote:
>>> > Eyal Birger <eyal.birger@gmail.com> wrote:
>>> >> This patch set introduces a new socket option for fetching the mark
>>> >> of skbs passed to sockets as ancillary data.
>>> >>
>>> >> A userspace program may wish to receive the mark of packets it
>>> >> receives, for example for distinguishing between different TPROXY
>>> >> diversion rules to the same userspace proxy socket.
>>> >
>>> > Hmm... Whats the use case?
>>> > Even if you cannot use multiple sockets for every divert rule,
>>> > TPROXY doesn't mangle payload; applications could use sockaddrs
>>> > returned by accept, getpeername, getsockname etc.  to figure out
>>> > which original port/address the packet was sent to?
>>>
>>> Right. But that would mean the criteria for traffic diversion would need to
>>> be known to the application receiving the traffic.
>>
>> For your solution to work the application needs to know about the TPROXY
>> rule set and how that is structured, no?
>>
>> I don't see how that is 'better' than e.g. looking at dst port number.
>  ...
>>> For example, a user space daemon can receive traffic from multiple
>>> applications using a single socket and distinguish between different traffic groups
>>> according to the packet mark.
>>
>> Right, but it might as well use SO_PEERCRED to identify the other pid, right?
>
> Also, this points out that this socket option if accepted probably needs to
> be privileged.
>
> I can see administrators not being happy with applications being able to see
> the marks used by their rulesets.

Thanks. I'll add that.

>
> I'm backing out this series, Florian makes a lot of good points.

I can sum up the motivation for this feature as follows:

- skb->mark is set by user-space policy. It would make sense for
user-space to be
  able to query the resolution of that policy on a per packet basis -
even if solely for the
  purpose of debugging (e.g. fetching this meta-data on packet sockets
on the xmit path
  in order to debug tc behavior)

- skb->mark allows decoupling between match criteria and flow
semantics in TPROXY
  et al. For example, in cases where the match criteria is based on
deep packet inspection
  or a combination of packet parameters, the matching logic has to be
duplicated in an
  application receiving several flows in a single socket

- It allows the use of skb->mark for light-weight segmentation of
traffic groups within
  a single namespace. skb->mark can be used in route rules and may be
set from an
  application. A missing piece is receiving the skb->mark in
interested applications.
  Granted, such use-case mandates a moderated environment.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 20:38         ` Eyal Birger
@ 2015-03-02 20:57           ` David Miller
  2015-03-02 21:11             ` Eyal Birger
  0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2015-03-02 20:57 UTC (permalink / raw)
  To: eyal.birger; +Cc: fw, willemb, edumazet, shmulik.ladkani, netdev

From: Eyal Birger <eyal.birger@gmail.com>
Date: Mon, 2 Mar 2015 22:38:50 +0200

> I can sum up the motivation for this feature as follows:
> 
> - skb->mark is set by user-space policy. It would make sense for
> user-space to be
>   able to query the resolution of that policy on a per packet basis -
> even if solely for the
>   purpose of debugging (e.g. fetching this meta-data on packet sockets
> on the xmit path
>   in order to debug tc behavior)

It is also set by routing, netfilter, classifier rules installed by the
administrator.

Therefore it is not universally true that it is safe to allow applications
to access this value.

I'm not applying this series, it's use case is at best dubious to me and
there are security/protection concerns as well.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 20:57           ` David Miller
@ 2015-03-02 21:11             ` Eyal Birger
  2015-03-02 21:45               ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Eyal Birger @ 2015-03-02 21:11 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Westphal, Willem de Bruijn, Eric Dumazet, Shmulik Ladkani,
	netdev@vger.kernel.org

On Mon, Mar 2, 2015 at 10:57 PM, David Miller <davem@davemloft.net> wrote:
> From: Eyal Birger <eyal.birger@gmail.com>
> Date: Mon, 2 Mar 2015 22:38:50 +0200
>
>> I can sum up the motivation for this feature as follows:
>>
>> - skb->mark is set by user-space policy. It would make sense for
>> user-space to be
>>   able to query the resolution of that policy on a per packet basis -
>> even if solely for the
>>   purpose of debugging (e.g. fetching this meta-data on packet sockets
>> on the xmit path
>>   in order to debug tc behavior)
>
> It is also set by routing, netfilter, classifier rules installed by the
> administrator.
>
> Therefore it is not universally true that it is safe to allow applications
> to access this value.
>
> I'm not applying this series, it's use case is at best dubious to me and
> there are security/protection concerns as well.

I understand. Thanks.

Would it be considered if access to the mark value was under CAP_NET_ADMIN
similar to setting SO_MARK?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 21:11             ` Eyal Birger
@ 2015-03-02 21:45               ` David Miller
  2015-03-03  3:45                 ` Eyal Birger
  0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2015-03-02 21:45 UTC (permalink / raw)
  To: eyal.birger; +Cc: fw, willemb, edumazet, shmulik.ladkani, netdev

From: Eyal Birger <eyal.birger@gmail.com>
Date: Mon, 2 Mar 2015 23:11:28 +0200

> On Mon, Mar 2, 2015 at 10:57 PM, David Miller <davem@davemloft.net> wrote:
>> From: Eyal Birger <eyal.birger@gmail.com>
>> Date: Mon, 2 Mar 2015 22:38:50 +0200
>>
>>> I can sum up the motivation for this feature as follows:
>>>
>>> - skb->mark is set by user-space policy. It would make sense for
>>> user-space to be
>>>   able to query the resolution of that policy on a per packet basis -
>>> even if solely for the
>>>   purpose of debugging (e.g. fetching this meta-data on packet sockets
>>> on the xmit path
>>>   in order to debug tc behavior)
>>
>> It is also set by routing, netfilter, classifier rules installed by the
>> administrator.
>>
>> Therefore it is not universally true that it is safe to allow applications
>> to access this value.
>>
>> I'm not applying this series, it's use case is at best dubious to me and
>> there are security/protection concerns as well.
> 
> I understand. Thanks.
> 
> Would it be considered if access to the mark value was under CAP_NET_ADMIN
> similar to setting SO_MARK?

I said there are two issues blocking it's inclusion, so dealing with only
one of those is insufficient, right?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-02 21:45               ` David Miller
@ 2015-03-03  3:45                 ` Eyal Birger
  2015-03-03  4:01                   ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Eyal Birger @ 2015-03-03  3:45 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Westphal, Willem de Bruijn, Eric Dumazet, Shmulik Ladkani,
	netdev@vger.kernel.org

>>> I'm not applying this series, it's use case is at best dubious to me and
>>> there are security/protection concerns as well.
>>
>> I understand. Thanks.
>>
>> Would it be considered if access to the mark value was under CAP_NET_ADMIN
>> similar to setting SO_MARK?
>
> I said there are two issues blocking it's inclusion, so dealing with only
> one of those is insufficient, right?

Right. Thanks.

In that case I suggest reverting 744d5a3e9fe2690
("net: move skb->dropcount to skb->cb[]") as it has no justification without
this feature and wastes 4 bytes of skb->cb[] space.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option
  2015-03-03  3:45                 ` Eyal Birger
@ 2015-03-03  4:01                   ` David Miller
  0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2015-03-03  4:01 UTC (permalink / raw)
  To: eyal.birger; +Cc: fw, willemb, edumazet, shmulik.ladkani, netdev

From: Eyal Birger <eyal.birger@gmail.com>
Date: Tue, 3 Mar 2015 05:45:39 +0200

>>>> I'm not applying this series, it's use case is at best dubious to me and
>>>> there are security/protection concerns as well.
>>>
>>> I understand. Thanks.
>>>
>>> Would it be considered if access to the mark value was under CAP_NET_ADMIN
>>> similar to setting SO_MARK?
>>
>> I said there are two issues blocking it's inclusion, so dealing with only
>> one of those is insufficient, right?
> 
> Right. Thanks.
> 
> In that case I suggest reverting 744d5a3e9fe2690
> ("net: move skb->dropcount to skb->cb[]") as it has no justification without
> this feature and wastes 4 bytes of skb->cb[] space.

It may allow us to make other simplifications to sk_buff so I'm
keeping it in.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-03-03  4:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-02 13:14 [PATCH net-next v4 0/2] net: Introducing socket mark receive socket option Eyal Birger
2015-03-02 13:14 ` [PATCH net-next v4 1/2] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv() Eyal Birger
2015-03-02 13:14 ` [PATCH net-next v4 2/2] net: Introducing socket mark receive socket option Eyal Birger
2015-03-02 13:29 ` [PATCH net-next v4 0/2] " Florian Westphal
2015-03-02 13:48   ` Eyal Birger
2015-03-02 14:36     ` Florian Westphal
2015-03-02 18:34       ` Eyal Birger
2015-03-02 18:55         ` Florian Westphal
2015-03-02 20:05       ` David Miller
2015-03-02 20:38         ` Eyal Birger
2015-03-02 20:57           ` David Miller
2015-03-02 21:11             ` Eyal Birger
2015-03-02 21:45               ` David Miller
2015-03-03  3:45                 ` Eyal Birger
2015-03-03  4:01                   ` David Miller
2015-03-02 20:01 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).