Development discussion of WireGuard
 help / color / mirror / Atom feed
* [Patch net] wireguard: preserve skb->mark on ingress side
@ 2021-09-28  3:19 Cong Wang
  2021-09-28  3:22 ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Cong Wang @ 2021-09-28  3:19 UTC (permalink / raw)
  To: netdev; +Cc: wireguard, Cong Wang, Peilin Ye, Jason A. Donenfeld

From: Cong Wang <cong.wang@bytedance.com>

On ingress side, wg_reset_packet() resets skb->mark twice: with
skb_scrub_packet() (xnet==true) and with memset() following it. But
skb->mark does not have to be cleared at least when staying in the
same net namespace, and other tunnels preserve it too similarly,
especially vxlan.

In our use case, we would like to preserve this skb->mark to
distinguish which wireguard device the packets are routed from.

Tested-by: Peilin Ye <peilin.ye@bytedance.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 drivers/net/wireguard/queueing.h | 9 +++++++--
 drivers/net/wireguard/receive.c  | 2 +-
 drivers/net/wireguard/send.c     | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
index 4ef2944a68bc..3516c1c59df0 100644
--- a/drivers/net/wireguard/queueing.h
+++ b/drivers/net/wireguard/queueing.h
@@ -73,15 +73,20 @@ static inline bool wg_check_packet_protocol(struct sk_buff *skb)
 	return real_protocol && skb->protocol == real_protocol;
 }
 
-static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
+static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating,
+				   bool xnet)
 {
 	u8 l4_hash = skb->l4_hash;
 	u8 sw_hash = skb->sw_hash;
 	u32 hash = skb->hash;
-	skb_scrub_packet(skb, true);
+	u32 mark;
+
+	skb_scrub_packet(skb, xnet);
+	mark = skb->mark;
 	memset(&skb->headers_start, 0,
 	       offsetof(struct sk_buff, headers_end) -
 		       offsetof(struct sk_buff, headers_start));
+	skb->mark = mark;
 	if (encapsulating) {
 		skb->l4_hash = l4_hash;
 		skb->sw_hash = sw_hash;
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 7dc84bcca261..385b2b60cfd9 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -476,7 +476,7 @@ int wg_packet_rx_poll(struct napi_struct *napi, int budget)
 		if (unlikely(wg_socket_endpoint_from_skb(&endpoint, skb)))
 			goto next;
 
-		wg_reset_packet(skb, false);
+		wg_reset_packet(skb, false, !net_eq(dev_net(peer->device->dev), dev_net(skb->dev)));
 		wg_packet_consume_data_done(peer, skb, &endpoint);
 		free = false;
 
diff --git a/drivers/net/wireguard/send.c b/drivers/net/wireguard/send.c
index 5368f7c35b4b..c77ef0815c2e 100644
--- a/drivers/net/wireguard/send.c
+++ b/drivers/net/wireguard/send.c
@@ -296,7 +296,7 @@ void wg_packet_encrypt_worker(struct work_struct *work)
 		skb_list_walk_safe(first, skb, next) {
 			if (likely(encrypt_packet(skb,
 					PACKET_CB(first)->keypair))) {
-				wg_reset_packet(skb, true);
+				wg_reset_packet(skb, true, true);
 			} else {
 				state = PACKET_STATE_DEAD;
 				break;
-- 
2.30.2


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

* Re: [Patch net] wireguard: preserve skb->mark on ingress side
  2021-09-28  3:19 [Patch net] wireguard: preserve skb->mark on ingress side Cong Wang
@ 2021-09-28  3:22 ` Jason A. Donenfeld
  2021-09-28  3:27   ` Cong Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2021-09-28  3:22 UTC (permalink / raw)
  To: Cong Wang; +Cc: Netdev, WireGuard mailing list, Cong Wang, Peilin Ye

Hi Cong,

I'm not so sure this makes sense, as the inner packet is in fact
totally different. If you want to distinguish the ingress interface,
can't you just use `iptables -i wg0` or `ip rule add ... iif wg0`?

Jason

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

* Re: [Patch net] wireguard: preserve skb->mark on ingress side
  2021-09-28  3:22 ` Jason A. Donenfeld
@ 2021-09-28  3:27   ` Cong Wang
  2021-10-07 20:55     ` Cong Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Cong Wang @ 2021-09-28  3:27 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Netdev, WireGuard mailing list, Cong Wang, Peilin Ye

On Mon, Sep 27, 2021 at 8:22 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Cong,
>
> I'm not so sure this makes sense, as the inner packet is in fact
> totally different. If you want to distinguish the ingress interface,

The contents are definitely different, but skb itself is the same.

Please also take a look at other tunnels, they all preserve this
in similar ways, that is, comparing net namespaces. Any reason
why wireguard is so different from other tunnels?

> can't you just use `iptables -i wg0` or `ip rule add ... iif wg0`?
>

My bad, I forgot to mention we run eBPF on egress side, where
skb->dev is already set to egress device (a non-wireguard device),
and of course skb_iif has been cleared even earlier.

Thanks.

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

* Re: [Patch net] wireguard: preserve skb->mark on ingress side
  2021-09-28  3:27   ` Cong Wang
@ 2021-10-07 20:55     ` Cong Wang
  0 siblings, 0 replies; 4+ messages in thread
From: Cong Wang @ 2021-10-07 20:55 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Netdev, WireGuard mailing list, Cong Wang, Peilin Ye

Hi, Jason

On Mon, Sep 27, 2021 at 8:27 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Sep 27, 2021 at 8:22 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > Hi Cong,
> >
> > I'm not so sure this makes sense, as the inner packet is in fact
> > totally different. If you want to distinguish the ingress interface,
>
> The contents are definitely different, but skb itself is the same.
>
> Please also take a look at other tunnels, they all preserve this
> in similar ways, that is, comparing net namespaces. Any reason
> why wireguard is so different from other tunnels?

Any response?

Thanks.

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

end of thread, other threads:[~2021-10-07 20:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28  3:19 [Patch net] wireguard: preserve skb->mark on ingress side Cong Wang
2021-09-28  3:22 ` Jason A. Donenfeld
2021-09-28  3:27   ` Cong Wang
2021-10-07 20:55     ` Cong Wang

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).