netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ipsec-next v2 0/3] xfrm: policy: replace session decode with flow dissector
@ 2023-09-29 12:58 Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Florian Westphal @ 2023-09-29 12:58 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, herbert, Florian Westphal

Remove the ipv4+ipv6 session decode functions and use generic flow
dissector to populate the flowi for the policy lookup.

Changes since v1:
- Can't use skb_flow_dissect(), we might see skbs that have neither
  skb->sk nor skb->dev set. Flow dissector WARN()s in this case, it
  tries to check for a bpf program assigned in that net namespace.

Add a preparation patch to pass down 'struct net' in
xfrm_decode_session so its available for use in patch 3.

Changes since RFC:

 - Drop mobility header support.  I don't think that anyone uses
   this.  MOBIKE doesn't appear to need this either.
 - Drop fl6->flowlabel assignment, original code leaves it as 0.

There is no reason for this change other than to remove code.

Florian Westphal (3):
  xfrm: pass struct net to xfrm_decode_session wrappers
  xfrm: move mark and oif flowi decode into common code
  xfrm: policy: replace session decode with flow dissector

 include/net/xfrm.h             |  10 +-
 net/ipv4/icmp.c                |   2 +-
 net/ipv4/ip_vti.c              |   4 +-
 net/ipv4/netfilter.c           |   2 +-
 net/ipv6/icmp.c                |   2 +-
 net/ipv6/ip6_vti.c             |   4 +-
 net/ipv6/netfilter.c           |   2 +-
 net/netfilter/nf_nat_proto.c   |   2 +-
 net/xfrm/xfrm_interface_core.c |   4 +-
 net/xfrm/xfrm_policy.c         | 287 +++++++++++++--------------------
 10 files changed, 128 insertions(+), 191 deletions(-)

-- 
2.41.0


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

* [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
  2023-09-29 12:58 [PATCH ipsec-next v2 0/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
@ 2023-09-29 12:58 ` Florian Westphal
  2023-09-29 22:42   ` kernel test robot
                     ` (2 more replies)
  2023-09-29 12:58 ` [PATCH ipsec-next v2 2/3] xfrm: move mark and oif flowi decode into common code Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 3/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
  2 siblings, 3 replies; 8+ messages in thread
From: Florian Westphal @ 2023-09-29 12:58 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, herbert, Florian Westphal, kernel test robot

Preparation patch, extra arg is not used.
No functional changes intended.

This is needed to replace the xfrm session decode functions with
the flow dissector.

skb_flow_dissect() cannot be used as-is, because it attempts to deduce the
'struct net' to use for bpf program fetch from skb->sk or skb->dev, but
xfrm code path can see skbs that have neither sk or dev filled in.

So either flow dissector needs to try harder, e.g. by also trying
skb->dst->dev, or we have to pass the struct net explicitly.

Passing the struct net doesn't look too bad to me, most places
already have it available or can derive it from the output device.

Reported-by: kernel test robot <oliver.sang@intel.com>
Link: https://lore.kernel.org/netdev/202309271628.27fd2187-oliver.sang@intel.com/
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: new in this series.

 include/net/xfrm.h             | 10 +++++-----
 net/ipv4/icmp.c                |  2 +-
 net/ipv4/ip_vti.c              |  4 ++--
 net/ipv4/netfilter.c           |  2 +-
 net/ipv6/icmp.c                |  2 +-
 net/ipv6/ip6_vti.c             |  4 ++--
 net/ipv6/netfilter.c           |  2 +-
 net/netfilter/nf_nat_proto.c   |  2 +-
 net/xfrm/xfrm_interface_core.c |  4 ++--
 net/xfrm/xfrm_policy.c         | 10 +++++-----
 10 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 35749a672cd1..474b2aed6a7f 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1207,20 +1207,20 @@ static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir,
 	return __xfrm_policy_check2(sk, dir, skb, AF_INET6, 1);
 }
 
-int __xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
+int __xfrm_decode_session(struct net *net, struct sk_buff *skb, struct flowi *fl,
 			  unsigned int family, int reverse);
 
-static inline int xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
+static inline int xfrm_decode_session(struct net *net, struct sk_buff *skb, struct flowi *fl,
 				      unsigned int family)
 {
-	return __xfrm_decode_session(skb, fl, family, 0);
+	return __xfrm_decode_session(net, skb, fl, family, 0);
 }
 
-static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
+static inline int xfrm_decode_session_reverse(struct net *net, struct sk_buff *skb,
 					      struct flowi *fl,
 					      unsigned int family)
 {
-	return __xfrm_decode_session(skb, fl, family, 1);
+	return __xfrm_decode_session(net, skb, fl, family, 1);
 }
 
 int __xfrm_route_forward(struct sk_buff *skb, unsigned short family);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index b8607763d113..e63a3bf99617 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -517,7 +517,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
 	} else
 		return rt;
 
-	err = xfrm_decode_session_reverse(skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
+	err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
 	if (err)
 		goto relookup_failed;
 
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index d1e7d0ceb7ed..9ab9b3ebe0cd 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -288,11 +288,11 @@ static netdev_tx_t vti_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
 		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET);
 		break;
 	case htons(ETH_P_IPV6):
 		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET6);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET6);
 		break;
 	default:
 		goto tx_err;
diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index bd135165482a..591a2737808e 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -62,7 +62,7 @@ int ip_route_me_harder(struct net *net, struct sock *sk, struct sk_buff *skb, un
 
 #ifdef CONFIG_XFRM
 	if (!(IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED) &&
-	    xfrm_decode_session(skb, flowi4_to_flowi(&fl4), AF_INET) == 0) {
+	    xfrm_decode_session(net, skb, flowi4_to_flowi(&fl4), AF_INET) == 0) {
 		struct dst_entry *dst = skb_dst(skb);
 		skb_dst_set(skb, NULL);
 		dst = xfrm_lookup(net, dst, flowi4_to_flowi(&fl4), sk, 0);
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 8fb4a791881a..f62427097126 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -385,7 +385,7 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net,
 			return dst;
 	}
 
-	err = xfrm_decode_session_reverse(skb, flowi6_to_flowi(&fl2), AF_INET6);
+	err = xfrm_decode_session_reverse(net, skb, flowi6_to_flowi(&fl2), AF_INET6);
 	if (err)
 		goto relookup_failed;
 
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 73c85d4e0e9c..e550240c85e1 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -569,11 +569,11 @@ vti6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 			goto tx_err;
 
 		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET6);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET6);
 		break;
 	case htons(ETH_P_IP):
 		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET);
 		break;
 	default:
 		goto tx_err;
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 857713d7a38a..53d255838e6a 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -61,7 +61,7 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
 
 #ifdef CONFIG_XFRM
 	if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
-	    xfrm_decode_session(skb, flowi6_to_flowi(&fl6), AF_INET6) == 0) {
+	    xfrm_decode_session(net, skb, flowi6_to_flowi(&fl6), AF_INET6) == 0) {
 		skb_dst_set(skb, NULL);
 		dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), sk, 0);
 		if (IS_ERR(dst))
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 48cc60084d28..c77963517bf8 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -668,7 +668,7 @@ static int nf_xfrm_me_harder(struct net *net, struct sk_buff *skb, unsigned int
 	struct flowi fl;
 	int err;
 
-	err = xfrm_decode_session(skb, &fl, family);
+	err = xfrm_decode_session(net, skb, &fl, family);
 	if (err < 0)
 		return err;
 
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index b86474084690..656f437f5f53 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -538,7 +538,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
 	switch (skb->protocol) {
 	case htons(ETH_P_IPV6):
 		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET6);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET6);
 		if (!dst) {
 			fl.u.ip6.flowi6_oif = dev->ifindex;
 			fl.u.ip6.flowi6_flags |= FLOWI_FLAG_ANYSRC;
@@ -553,7 +553,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
 		break;
 	case htons(ETH_P_IP):
 		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
-		xfrm_decode_session(skb, &fl, AF_INET);
+		xfrm_decode_session(dev_net(dev), skb, &fl, AF_INET);
 		if (!dst) {
 			struct rtable *rt;
 
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index c4c4fc29ccf5..064d1744fa36 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2853,7 +2853,7 @@ static void xfrm_policy_queue_process(struct timer_list *t)
 	/* Fixup the mark to support VTI. */
 	skb_mark = skb->mark;
 	skb->mark = pol->mark.v;
-	xfrm_decode_session(skb, &fl, dst->ops->family);
+	xfrm_decode_session(net, skb, &fl, dst->ops->family);
 	skb->mark = skb_mark;
 	spin_unlock(&pq->hold_queue.lock);
 
@@ -2889,7 +2889,7 @@ static void xfrm_policy_queue_process(struct timer_list *t)
 		/* Fixup the mark to support VTI. */
 		skb_mark = skb->mark;
 		skb->mark = pol->mark.v;
-		xfrm_decode_session(skb, &fl, skb_dst(skb)->ops->family);
+		xfrm_decode_session(net, skb, &fl, skb_dst(skb)->ops->family);
 		skb->mark = skb_mark;
 
 		dst_hold(xfrm_dst_path(skb_dst(skb)));
@@ -3554,7 +3554,7 @@ decode_session6(struct sk_buff *skb, struct flowi *fl, bool reverse)
 }
 #endif
 
-int __xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
+int __xfrm_decode_session(struct net *net, struct sk_buff *skb, struct flowi *fl,
 			  unsigned int family, int reverse)
 {
 	switch (family) {
@@ -3618,7 +3618,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
 	reverse = dir & ~XFRM_POLICY_MASK;
 	dir &= XFRM_POLICY_MASK;
 
-	if (__xfrm_decode_session(skb, &fl, family, reverse) < 0) {
+	if (__xfrm_decode_session(net, skb, &fl, family, reverse) < 0) {
 		XFRM_INC_STATS(net, LINUX_MIB_XFRMINHDRERROR);
 		return 0;
 	}
@@ -3774,7 +3774,7 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	struct dst_entry *dst;
 	int res = 1;
 
-	if (xfrm_decode_session(skb, &fl, family) < 0) {
+	if (xfrm_decode_session(net, skb, &fl, family) < 0) {
 		XFRM_INC_STATS(net, LINUX_MIB_XFRMFWDHDRERROR);
 		return 0;
 	}
-- 
2.41.0


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

* [PATCH ipsec-next v2 2/3] xfrm: move mark and oif flowi decode into common code
  2023-09-29 12:58 [PATCH ipsec-next v2 0/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
@ 2023-09-29 12:58 ` Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 3/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
  2 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2023-09-29 12:58 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, herbert, Florian Westphal, Simon Horman

flowi4_oif/flowi6_oif and mark are aliased to flowi_common field, i.e.
all can be used interchangeably.

Instead of duplicating place this in common code.
No functional changes intended.

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 no changes since last version, keeping Simons RvB tag.

 net/xfrm/xfrm_policy.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 064d1744fa36..da29be0b002f 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3373,14 +3373,8 @@ decode_session4(struct sk_buff *skb, struct flowi *fl, bool reverse)
 	int ihl = iph->ihl;
 	u8 *xprth = skb_network_header(skb) + ihl * 4;
 	struct flowi4 *fl4 = &fl->u.ip4;
-	int oif = 0;
-
-	if (skb_dst(skb) && skb_dst(skb)->dev)
-		oif = skb_dst(skb)->dev->ifindex;
 
 	memset(fl4, 0, sizeof(struct flowi4));
-	fl4->flowi4_mark = skb->mark;
-	fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
 
 	fl4->flowi4_proto = iph->protocol;
 	fl4->daddr = reverse ? iph->saddr : iph->daddr;
@@ -3451,7 +3445,6 @@ decode_session6(struct sk_buff *skb, struct flowi *fl, bool reverse)
 	struct ipv6_opt_hdr *exthdr;
 	const unsigned char *nh = skb_network_header(skb);
 	u16 nhoff = IP6CB(skb)->nhoff;
-	int oif = 0;
 	u8 nexthdr;
 
 	if (!nhoff)
@@ -3459,12 +3452,7 @@ decode_session6(struct sk_buff *skb, struct flowi *fl, bool reverse)
 
 	nexthdr = nh[nhoff];
 
-	if (skb_dst(skb) && skb_dst(skb)->dev)
-		oif = skb_dst(skb)->dev->ifindex;
-
 	memset(fl6, 0, sizeof(struct flowi6));
-	fl6->flowi6_mark = skb->mark;
-	fl6->flowi6_oif = reverse ? skb->skb_iif : oif;
 
 	fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
 	fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
@@ -3570,6 +3558,18 @@ int __xfrm_decode_session(struct net *net, struct sk_buff *skb, struct flowi *fl
 		return -EAFNOSUPPORT;
 	}
 
+	fl->flowi_mark = skb->mark;
+	if (reverse) {
+		fl->flowi_oif = skb->skb_iif;
+	} else {
+		int oif = 0;
+
+		if (skb_dst(skb) && skb_dst(skb)->dev)
+			oif = skb_dst(skb)->dev->ifindex;
+
+		fl->flowi_oif = oif;
+	}
+
 	return security_xfrm_decode_session(skb, &fl->flowi_secid);
 }
 EXPORT_SYMBOL(__xfrm_decode_session);
-- 
2.41.0


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

* [PATCH ipsec-next v2 3/3] xfrm: policy: replace session decode with flow dissector
  2023-09-29 12:58 [PATCH ipsec-next v2 0/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
  2023-09-29 12:58 ` [PATCH ipsec-next v2 2/3] xfrm: move mark and oif flowi decode into common code Florian Westphal
@ 2023-09-29 12:58 ` Florian Westphal
  2 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2023-09-29 12:58 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, herbert, Florian Westphal, Simon Horman

xfrm needs to populate ipv4/v6 flow struct for route lookup.
In the past there were several bugs in this code:

1. callers that forget to reload header pointers after
   xfrm_decode_session() (it may pull headers).
2. bugs in decoding where accesses past skb->data occurred.

Meanwhile network core gained a packet dissector as well.
This switches xfrm to the flow dissector.

Changes since RFC:
Drop ipv6 mobiliy header support, AFAIU noone uses this.

Drop extraction of flowlabel, replaced code doesn't set it either.

Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://lore.kernel.org/netdev/20230908120628.26164-3-fw@strlen.de/
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Change since last version:
 use __skb_flow_dissect() instead of skb_flow_dissect(), we need to pass
 struct net along.  No other changes, retaining Simons RvB tag.

 net/xfrm/xfrm_policy.c | 253 ++++++++++++++++-------------------------
 1 file changed, 95 insertions(+), 158 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index da29be0b002f..6aea8b2f45e0 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -149,6 +149,21 @@ struct xfrm_pol_inexact_candidates {
 	struct hlist_head *res[XFRM_POL_CAND_MAX];
 };
 
+struct xfrm_flow_keys {
+	struct flow_dissector_key_basic basic;
+	struct flow_dissector_key_control control;
+	union {
+		struct flow_dissector_key_ipv4_addrs ipv4;
+		struct flow_dissector_key_ipv6_addrs ipv6;
+	} addrs;
+	struct flow_dissector_key_ip ip;
+	struct flow_dissector_key_icmp icmp;
+	struct flow_dissector_key_ports ports;
+	struct flow_dissector_key_keyid gre;
+};
+
+static struct flow_dissector xfrm_session_dissector __ro_after_init;
+
 static DEFINE_SPINLOCK(xfrm_if_cb_lock);
 static struct xfrm_if_cb const __rcu *xfrm_if_cb __read_mostly;
 
@@ -3367,191 +3382,74 @@ xfrm_policy_ok(const struct xfrm_tmpl *tmpl, const struct sec_path *sp, int star
 }
 
 static void
-decode_session4(struct sk_buff *skb, struct flowi *fl, bool reverse)
+decode_session4(const struct xfrm_flow_keys *flkeys, struct flowi *fl, bool reverse)
 {
-	const struct iphdr *iph = ip_hdr(skb);
-	int ihl = iph->ihl;
-	u8 *xprth = skb_network_header(skb) + ihl * 4;
 	struct flowi4 *fl4 = &fl->u.ip4;
 
 	memset(fl4, 0, sizeof(struct flowi4));
 
-	fl4->flowi4_proto = iph->protocol;
-	fl4->daddr = reverse ? iph->saddr : iph->daddr;
-	fl4->saddr = reverse ? iph->daddr : iph->saddr;
-	fl4->flowi4_tos = iph->tos & ~INET_ECN_MASK;
-
-	if (!ip_is_fragment(iph)) {
-		switch (iph->protocol) {
-		case IPPROTO_UDP:
-		case IPPROTO_UDPLITE:
-		case IPPROTO_TCP:
-		case IPPROTO_SCTP:
-		case IPPROTO_DCCP:
-			if (xprth + 4 < skb->data ||
-			    pskb_may_pull(skb, xprth + 4 - skb->data)) {
-				__be16 *ports;
-
-				xprth = skb_network_header(skb) + ihl * 4;
-				ports = (__be16 *)xprth;
-
-				fl4->fl4_sport = ports[!!reverse];
-				fl4->fl4_dport = ports[!reverse];
-			}
-			break;
-		case IPPROTO_ICMP:
-			if (xprth + 2 < skb->data ||
-			    pskb_may_pull(skb, xprth + 2 - skb->data)) {
-				u8 *icmp;
-
-				xprth = skb_network_header(skb) + ihl * 4;
-				icmp = xprth;
-
-				fl4->fl4_icmp_type = icmp[0];
-				fl4->fl4_icmp_code = icmp[1];
-			}
-			break;
-		case IPPROTO_GRE:
-			if (xprth + 12 < skb->data ||
-			    pskb_may_pull(skb, xprth + 12 - skb->data)) {
-				__be16 *greflags;
-				__be32 *gre_hdr;
-
-				xprth = skb_network_header(skb) + ihl * 4;
-				greflags = (__be16 *)xprth;
-				gre_hdr = (__be32 *)xprth;
-
-				if (greflags[0] & GRE_KEY) {
-					if (greflags[0] & GRE_CSUM)
-						gre_hdr++;
-					fl4->fl4_gre_key = gre_hdr[1];
-				}
-			}
-			break;
-		default:
-			break;
-		}
+	if (reverse) {
+		fl4->saddr = flkeys->addrs.ipv4.dst;
+		fl4->daddr = flkeys->addrs.ipv4.src;
+		fl4->fl4_sport = flkeys->ports.dst;
+		fl4->fl4_dport = flkeys->ports.src;
+	} else {
+		fl4->saddr = flkeys->addrs.ipv4.src;
+		fl4->daddr = flkeys->addrs.ipv4.dst;
+		fl4->fl4_sport = flkeys->ports.src;
+		fl4->fl4_dport = flkeys->ports.dst;
 	}
+
+	fl4->flowi4_proto = flkeys->basic.ip_proto;
+	fl4->flowi4_tos = flkeys->ip.tos;
+	fl4->fl4_icmp_type = flkeys->icmp.type;
+	fl4->fl4_icmp_type = flkeys->icmp.code;
+	fl4->fl4_gre_key = flkeys->gre.keyid;
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
 static void
-decode_session6(struct sk_buff *skb, struct flowi *fl, bool reverse)
+decode_session6(const struct xfrm_flow_keys *flkeys, struct flowi *fl, bool reverse)
 {
 	struct flowi6 *fl6 = &fl->u.ip6;
-	int onlyproto = 0;
-	const struct ipv6hdr *hdr = ipv6_hdr(skb);
-	u32 offset = sizeof(*hdr);
-	struct ipv6_opt_hdr *exthdr;
-	const unsigned char *nh = skb_network_header(skb);
-	u16 nhoff = IP6CB(skb)->nhoff;
-	u8 nexthdr;
-
-	if (!nhoff)
-		nhoff = offsetof(struct ipv6hdr, nexthdr);
-
-	nexthdr = nh[nhoff];
 
 	memset(fl6, 0, sizeof(struct flowi6));
 
-	fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
-	fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
-
-	while (nh + offset + sizeof(*exthdr) < skb->data ||
-	       pskb_may_pull(skb, nh + offset + sizeof(*exthdr) - skb->data)) {
-		nh = skb_network_header(skb);
-		exthdr = (struct ipv6_opt_hdr *)(nh + offset);
-
-		switch (nexthdr) {
-		case NEXTHDR_FRAGMENT:
-			onlyproto = 1;
-			fallthrough;
-		case NEXTHDR_ROUTING:
-		case NEXTHDR_HOP:
-		case NEXTHDR_DEST:
-			offset += ipv6_optlen(exthdr);
-			nexthdr = exthdr->nexthdr;
-			break;
-		case IPPROTO_UDP:
-		case IPPROTO_UDPLITE:
-		case IPPROTO_TCP:
-		case IPPROTO_SCTP:
-		case IPPROTO_DCCP:
-			if (!onlyproto && (nh + offset + 4 < skb->data ||
-			     pskb_may_pull(skb, nh + offset + 4 - skb->data))) {
-				__be16 *ports;
-
-				nh = skb_network_header(skb);
-				ports = (__be16 *)(nh + offset);
-				fl6->fl6_sport = ports[!!reverse];
-				fl6->fl6_dport = ports[!reverse];
-			}
-			fl6->flowi6_proto = nexthdr;
-			return;
-		case IPPROTO_ICMPV6:
-			if (!onlyproto && (nh + offset + 2 < skb->data ||
-			    pskb_may_pull(skb, nh + offset + 2 - skb->data))) {
-				u8 *icmp;
-
-				nh = skb_network_header(skb);
-				icmp = (u8 *)(nh + offset);
-				fl6->fl6_icmp_type = icmp[0];
-				fl6->fl6_icmp_code = icmp[1];
-			}
-			fl6->flowi6_proto = nexthdr;
-			return;
-		case IPPROTO_GRE:
-			if (!onlyproto &&
-			    (nh + offset + 12 < skb->data ||
-			     pskb_may_pull(skb, nh + offset + 12 - skb->data))) {
-				struct gre_base_hdr *gre_hdr;
-				__be32 *gre_key;
-
-				nh = skb_network_header(skb);
-				gre_hdr = (struct gre_base_hdr *)(nh + offset);
-				gre_key = (__be32 *)(gre_hdr + 1);
-
-				if (gre_hdr->flags & GRE_KEY) {
-					if (gre_hdr->flags & GRE_CSUM)
-						gre_key++;
-					fl6->fl6_gre_key = *gre_key;
-				}
-			}
-			fl6->flowi6_proto = nexthdr;
-			return;
-
-#if IS_ENABLED(CONFIG_IPV6_MIP6)
-		case IPPROTO_MH:
-			offset += ipv6_optlen(exthdr);
-			if (!onlyproto && (nh + offset + 3 < skb->data ||
-			    pskb_may_pull(skb, nh + offset + 3 - skb->data))) {
-				struct ip6_mh *mh;
-
-				nh = skb_network_header(skb);
-				mh = (struct ip6_mh *)(nh + offset);
-				fl6->fl6_mh_type = mh->ip6mh_type;
-			}
-			fl6->flowi6_proto = nexthdr;
-			return;
-#endif
-		default:
-			fl6->flowi6_proto = nexthdr;
-			return;
-		}
+	if (reverse) {
+		fl6->saddr = flkeys->addrs.ipv6.dst;
+		fl6->daddr = flkeys->addrs.ipv6.src;
+		fl6->fl6_sport = flkeys->ports.dst;
+		fl6->fl6_dport = flkeys->ports.src;
+	} else {
+		fl6->saddr = flkeys->addrs.ipv6.src;
+		fl6->daddr = flkeys->addrs.ipv6.dst;
+		fl6->fl6_sport = flkeys->ports.src;
+		fl6->fl6_dport = flkeys->ports.dst;
 	}
+
+	fl6->flowi6_proto = flkeys->basic.ip_proto;
+	fl6->fl6_icmp_type = flkeys->icmp.type;
+	fl6->fl6_icmp_type = flkeys->icmp.code;
+	fl6->fl6_gre_key = flkeys->gre.keyid;
 }
 #endif
 
 int __xfrm_decode_session(struct net *net, struct sk_buff *skb, struct flowi *fl,
 			  unsigned int family, int reverse)
 {
+	struct xfrm_flow_keys flkeys;
+
+	memset(&flkeys, 0, sizeof(flkeys));
+	__skb_flow_dissect(net, skb, &xfrm_session_dissector, &flkeys,
+			   NULL, 0, 0, 0, FLOW_DISSECTOR_F_STOP_AT_ENCAP);
+
 	switch (family) {
 	case AF_INET:
-		decode_session4(skb, fl, reverse);
+		decode_session4(&flkeys, fl, reverse);
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
-		decode_session6(skb, fl, reverse);
+		decode_session6(&flkeys, fl, reverse);
 		break;
 #endif
 	default:
@@ -4253,8 +4151,47 @@ static struct pernet_operations __net_initdata xfrm_net_ops = {
 	.exit = xfrm_net_exit,
 };
 
+static const struct flow_dissector_key xfrm_flow_dissector_keys[] = {
+	{
+		.key_id = FLOW_DISSECTOR_KEY_CONTROL,
+		.offset = offsetof(struct xfrm_flow_keys, control),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_BASIC,
+		.offset = offsetof(struct xfrm_flow_keys, basic),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_IPV4_ADDRS,
+		.offset = offsetof(struct xfrm_flow_keys, addrs.ipv4),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_IPV6_ADDRS,
+		.offset = offsetof(struct xfrm_flow_keys, addrs.ipv6),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_PORTS,
+		.offset = offsetof(struct xfrm_flow_keys, ports),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_GRE_KEYID,
+		.offset = offsetof(struct xfrm_flow_keys, gre),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_IP,
+		.offset = offsetof(struct xfrm_flow_keys, ip),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_ICMP,
+		.offset = offsetof(struct xfrm_flow_keys, icmp),
+	},
+};
+
 void __init xfrm_init(void)
 {
+	skb_flow_dissector_init(&xfrm_session_dissector,
+				xfrm_flow_dissector_keys,
+				ARRAY_SIZE(xfrm_flow_dissector_keys));
+
 	register_pernet_subsys(&xfrm_net_ops);
 	xfrm_dev_init();
 	xfrm_input_init();
-- 
2.41.0


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

* Re: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
@ 2023-09-29 22:42   ` kernel test robot
  2023-09-30  7:51     ` Florian Westphal
  2023-09-29 22:52   ` kernel test robot
  2023-09-30  2:00   ` kernel test robot
  2 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2023-09-29 22:42 UTC (permalink / raw)
  To: Florian Westphal, netdev
  Cc: oe-kbuild-all, steffen.klassert, herbert, Florian Westphal,
	kernel test robot

Hi Florian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on klassert-ipsec-next/master]
[also build test WARNING on klassert-ipsec/master netfilter-nf/main linus/master v6.6-rc3 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/xfrm-pass-struct-net-to-xfrm_decode_session-wrappers/20230929-210047
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
patch link:    https://lore.kernel.org/r/20230929125848.5445-2-fw%40strlen.de
patch subject: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
config: microblaze-defconfig (https://download.01.org/0day-ci/archive/20230930/202309300634.DBomJJ9W-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309300634.DBomJJ9W-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309300634.DBomJJ9W-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/ipv4/icmp.c: In function 'icmp_route_lookup':
   net/ipv4/icmp.c:520:43: error: passing argument 1 of 'xfrm_decode_session_reverse' from incompatible pointer type [-Werror=incompatible-pointer-types]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                           ^~~
         |                                           |
         |                                           struct net *
   In file included from net/ipv4/icmp.c:91:
   include/net/xfrm.h:1299:63: note: expected 'struct sk_buff *' but argument is of type 'struct net *'
    1299 | static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
         |                                               ~~~~~~~~~~~~~~~~^~~
   net/ipv4/icmp.c:520:48: error: passing argument 2 of 'xfrm_decode_session_reverse' from incompatible pointer type [-Werror=incompatible-pointer-types]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                                ^~~~~~
         |                                                |
         |                                                struct sk_buff *
   include/net/xfrm.h:1300:61: note: expected 'struct flowi *' but argument is of type 'struct sk_buff *'
    1300 |                                               struct flowi *fl,
         |                                               ~~~~~~~~~~~~~~^~
>> net/ipv4/icmp.c:520:56: warning: passing argument 3 of 'xfrm_decode_session_reverse' makes integer from pointer without a cast [-Wint-conversion]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                                                        |
         |                                                        struct flowi *
   include/net/xfrm.h:1301:60: note: expected 'unsigned int' but argument is of type 'struct flowi *'
    1301 |                                               unsigned int family)
         |                                               ~~~~~~~~~~~~~^~~~~~
   net/ipv4/icmp.c:520:15: error: too many arguments to function 'xfrm_decode_session_reverse'
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/xfrm.h:1299:19: note: declared here
    1299 | static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/xfrm_decode_session_reverse +520 net/ipv4/icmp.c

   475	
   476	static struct rtable *icmp_route_lookup(struct net *net,
   477						struct flowi4 *fl4,
   478						struct sk_buff *skb_in,
   479						const struct iphdr *iph,
   480						__be32 saddr, u8 tos, u32 mark,
   481						int type, int code,
   482						struct icmp_bxm *param)
   483	{
   484		struct net_device *route_lookup_dev;
   485		struct rtable *rt, *rt2;
   486		struct flowi4 fl4_dec;
   487		int err;
   488	
   489		memset(fl4, 0, sizeof(*fl4));
   490		fl4->daddr = (param->replyopts.opt.opt.srr ?
   491			      param->replyopts.opt.opt.faddr : iph->saddr);
   492		fl4->saddr = saddr;
   493		fl4->flowi4_mark = mark;
   494		fl4->flowi4_uid = sock_net_uid(net, NULL);
   495		fl4->flowi4_tos = RT_TOS(tos);
   496		fl4->flowi4_proto = IPPROTO_ICMP;
   497		fl4->fl4_icmp_type = type;
   498		fl4->fl4_icmp_code = code;
   499		route_lookup_dev = icmp_get_route_lookup_dev(skb_in);
   500		fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
   501	
   502		security_skb_classify_flow(skb_in, flowi4_to_flowi_common(fl4));
   503		rt = ip_route_output_key_hash(net, fl4, skb_in);
   504		if (IS_ERR(rt))
   505			return rt;
   506	
   507		/* No need to clone since we're just using its address. */
   508		rt2 = rt;
   509	
   510		rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
   511						   flowi4_to_flowi(fl4), NULL, 0);
   512		if (!IS_ERR(rt)) {
   513			if (rt != rt2)
   514				return rt;
   515		} else if (PTR_ERR(rt) == -EPERM) {
   516			rt = NULL;
   517		} else
   518			return rt;
   519	
 > 520		err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
   521		if (err)
   522			goto relookup_failed;
   523	
   524		if (inet_addr_type_dev_table(net, route_lookup_dev,
   525					     fl4_dec.saddr) == RTN_LOCAL) {
   526			rt2 = __ip_route_output_key(net, &fl4_dec);
   527			if (IS_ERR(rt2))
   528				err = PTR_ERR(rt2);
   529		} else {
   530			struct flowi4 fl4_2 = {};
   531			unsigned long orefdst;
   532	
   533			fl4_2.daddr = fl4_dec.saddr;
   534			rt2 = ip_route_output_key(net, &fl4_2);
   535			if (IS_ERR(rt2)) {
   536				err = PTR_ERR(rt2);
   537				goto relookup_failed;
   538			}
   539			/* Ugh! */
   540			orefdst = skb_in->_skb_refdst; /* save old refdst */
   541			skb_dst_set(skb_in, NULL);
   542			err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
   543					     RT_TOS(tos), rt2->dst.dev);
   544	
   545			dst_release(&rt2->dst);
   546			rt2 = skb_rtable(skb_in);
   547			skb_in->_skb_refdst = orefdst; /* restore old refdst */
   548		}
   549	
   550		if (err)
   551			goto relookup_failed;
   552	
   553		rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst,
   554						    flowi4_to_flowi(&fl4_dec), NULL,
   555						    XFRM_LOOKUP_ICMP);
   556		if (!IS_ERR(rt2)) {
   557			dst_release(&rt->dst);
   558			memcpy(fl4, &fl4_dec, sizeof(*fl4));
   559			rt = rt2;
   560		} else if (PTR_ERR(rt2) == -EPERM) {
   561			if (rt)
   562				dst_release(&rt->dst);
   563			return rt2;
   564		} else {
   565			err = PTR_ERR(rt2);
   566			goto relookup_failed;
   567		}
   568		return rt;
   569	
   570	relookup_failed:
   571		if (rt)
   572			return rt;
   573		return ERR_PTR(err);
   574	}
   575	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
  2023-09-29 22:42   ` kernel test robot
@ 2023-09-29 22:52   ` kernel test robot
  2023-09-30  2:00   ` kernel test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-09-29 22:52 UTC (permalink / raw)
  To: Florian Westphal, netdev
  Cc: llvm, oe-kbuild-all, steffen.klassert, herbert, Florian Westphal,
	kernel test robot

Hi Florian,

kernel test robot noticed the following build errors:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master netfilter-nf/main linus/master v6.6-rc3 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/xfrm-pass-struct-net-to-xfrm_decode_session-wrappers/20230929-210047
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
patch link:    https://lore.kernel.org/r/20230929125848.5445-2-fw%40strlen.de
patch subject: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20230930/202309300637.sTEZ8Sa9-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309300637.sTEZ8Sa9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309300637.sTEZ8Sa9-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/ipv4/icmp.c:69:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from net/ipv4/icmp.c:69:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from net/ipv4/icmp.c:69:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> net/ipv4/icmp.c:520:76: error: too many arguments to function call, expected 3, have 4
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~                                         ^~~~~~~
   include/linux/socket.h:192:18: note: expanded from macro 'AF_INET'
     192 | #define AF_INET         2       /* Internet IP Protocol         */
         |                         ^
   include/net/xfrm.h:1299:19: note: 'xfrm_decode_session_reverse' declared here
    1299 | static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
         |                   ^
   12 warnings and 1 error generated.


vim +520 net/ipv4/icmp.c

   475	
   476	static struct rtable *icmp_route_lookup(struct net *net,
   477						struct flowi4 *fl4,
   478						struct sk_buff *skb_in,
   479						const struct iphdr *iph,
   480						__be32 saddr, u8 tos, u32 mark,
   481						int type, int code,
   482						struct icmp_bxm *param)
   483	{
   484		struct net_device *route_lookup_dev;
   485		struct rtable *rt, *rt2;
   486		struct flowi4 fl4_dec;
   487		int err;
   488	
   489		memset(fl4, 0, sizeof(*fl4));
   490		fl4->daddr = (param->replyopts.opt.opt.srr ?
   491			      param->replyopts.opt.opt.faddr : iph->saddr);
   492		fl4->saddr = saddr;
   493		fl4->flowi4_mark = mark;
   494		fl4->flowi4_uid = sock_net_uid(net, NULL);
   495		fl4->flowi4_tos = RT_TOS(tos);
   496		fl4->flowi4_proto = IPPROTO_ICMP;
   497		fl4->fl4_icmp_type = type;
   498		fl4->fl4_icmp_code = code;
   499		route_lookup_dev = icmp_get_route_lookup_dev(skb_in);
   500		fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
   501	
   502		security_skb_classify_flow(skb_in, flowi4_to_flowi_common(fl4));
   503		rt = ip_route_output_key_hash(net, fl4, skb_in);
   504		if (IS_ERR(rt))
   505			return rt;
   506	
   507		/* No need to clone since we're just using its address. */
   508		rt2 = rt;
   509	
   510		rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
   511						   flowi4_to_flowi(fl4), NULL, 0);
   512		if (!IS_ERR(rt)) {
   513			if (rt != rt2)
   514				return rt;
   515		} else if (PTR_ERR(rt) == -EPERM) {
   516			rt = NULL;
   517		} else
   518			return rt;
   519	
 > 520		err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
   521		if (err)
   522			goto relookup_failed;
   523	
   524		if (inet_addr_type_dev_table(net, route_lookup_dev,
   525					     fl4_dec.saddr) == RTN_LOCAL) {
   526			rt2 = __ip_route_output_key(net, &fl4_dec);
   527			if (IS_ERR(rt2))
   528				err = PTR_ERR(rt2);
   529		} else {
   530			struct flowi4 fl4_2 = {};
   531			unsigned long orefdst;
   532	
   533			fl4_2.daddr = fl4_dec.saddr;
   534			rt2 = ip_route_output_key(net, &fl4_2);
   535			if (IS_ERR(rt2)) {
   536				err = PTR_ERR(rt2);
   537				goto relookup_failed;
   538			}
   539			/* Ugh! */
   540			orefdst = skb_in->_skb_refdst; /* save old refdst */
   541			skb_dst_set(skb_in, NULL);
   542			err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
   543					     RT_TOS(tos), rt2->dst.dev);
   544	
   545			dst_release(&rt2->dst);
   546			rt2 = skb_rtable(skb_in);
   547			skb_in->_skb_refdst = orefdst; /* restore old refdst */
   548		}
   549	
   550		if (err)
   551			goto relookup_failed;
   552	
   553		rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst,
   554						    flowi4_to_flowi(&fl4_dec), NULL,
   555						    XFRM_LOOKUP_ICMP);
   556		if (!IS_ERR(rt2)) {
   557			dst_release(&rt->dst);
   558			memcpy(fl4, &fl4_dec, sizeof(*fl4));
   559			rt = rt2;
   560		} else if (PTR_ERR(rt2) == -EPERM) {
   561			if (rt)
   562				dst_release(&rt->dst);
   563			return rt2;
   564		} else {
   565			err = PTR_ERR(rt2);
   566			goto relookup_failed;
   567		}
   568		return rt;
   569	
   570	relookup_failed:
   571		if (rt)
   572			return rt;
   573		return ERR_PTR(err);
   574	}
   575	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
  2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
  2023-09-29 22:42   ` kernel test robot
  2023-09-29 22:52   ` kernel test robot
@ 2023-09-30  2:00   ` kernel test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-09-30  2:00 UTC (permalink / raw)
  To: Florian Westphal, netdev
  Cc: oe-kbuild-all, steffen.klassert, herbert, Florian Westphal,
	kernel test robot

Hi Florian,

kernel test robot noticed the following build errors:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master netfilter-nf/main linus/master v6.6-rc3 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/xfrm-pass-struct-net-to-xfrm_decode_session-wrappers/20230929-210047
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
patch link:    https://lore.kernel.org/r/20230929125848.5445-2-fw%40strlen.de
patch subject: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
config: microblaze-defconfig (https://download.01.org/0day-ci/archive/20230930/202309300951.j4h5KqUb-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309300951.j4h5KqUb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309300951.j4h5KqUb-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/ipv4/icmp.c: In function 'icmp_route_lookup':
>> net/ipv4/icmp.c:520:43: error: passing argument 1 of 'xfrm_decode_session_reverse' from incompatible pointer type [-Werror=incompatible-pointer-types]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                           ^~~
         |                                           |
         |                                           struct net *
   In file included from net/ipv4/icmp.c:91:
   include/net/xfrm.h:1299:63: note: expected 'struct sk_buff *' but argument is of type 'struct net *'
    1299 | static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
         |                                               ~~~~~~~~~~~~~~~~^~~
   net/ipv4/icmp.c:520:48: error: passing argument 2 of 'xfrm_decode_session_reverse' from incompatible pointer type [-Werror=incompatible-pointer-types]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                                ^~~~~~
         |                                                |
         |                                                struct sk_buff *
   include/net/xfrm.h:1300:61: note: expected 'struct flowi *' but argument is of type 'struct sk_buff *'
    1300 |                                               struct flowi *fl,
         |                                               ~~~~~~~~~~~~~~^~
   net/ipv4/icmp.c:520:56: warning: passing argument 3 of 'xfrm_decode_session_reverse' makes integer from pointer without a cast [-Wint-conversion]
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                                                        |
         |                                                        struct flowi *
   include/net/xfrm.h:1301:60: note: expected 'unsigned int' but argument is of type 'struct flowi *'
    1301 |                                               unsigned int family)
         |                                               ~~~~~~~~~~~~~^~~~~~
>> net/ipv4/icmp.c:520:15: error: too many arguments to function 'xfrm_decode_session_reverse'
     520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/xfrm.h:1299:19: note: declared here
    1299 | static inline int xfrm_decode_session_reverse(struct sk_buff *skb,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/xfrm_decode_session_reverse +520 net/ipv4/icmp.c

   475	
   476	static struct rtable *icmp_route_lookup(struct net *net,
   477						struct flowi4 *fl4,
   478						struct sk_buff *skb_in,
   479						const struct iphdr *iph,
   480						__be32 saddr, u8 tos, u32 mark,
   481						int type, int code,
   482						struct icmp_bxm *param)
   483	{
   484		struct net_device *route_lookup_dev;
   485		struct rtable *rt, *rt2;
   486		struct flowi4 fl4_dec;
   487		int err;
   488	
   489		memset(fl4, 0, sizeof(*fl4));
   490		fl4->daddr = (param->replyopts.opt.opt.srr ?
   491			      param->replyopts.opt.opt.faddr : iph->saddr);
   492		fl4->saddr = saddr;
   493		fl4->flowi4_mark = mark;
   494		fl4->flowi4_uid = sock_net_uid(net, NULL);
   495		fl4->flowi4_tos = RT_TOS(tos);
   496		fl4->flowi4_proto = IPPROTO_ICMP;
   497		fl4->fl4_icmp_type = type;
   498		fl4->fl4_icmp_code = code;
   499		route_lookup_dev = icmp_get_route_lookup_dev(skb_in);
   500		fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
   501	
   502		security_skb_classify_flow(skb_in, flowi4_to_flowi_common(fl4));
   503		rt = ip_route_output_key_hash(net, fl4, skb_in);
   504		if (IS_ERR(rt))
   505			return rt;
   506	
   507		/* No need to clone since we're just using its address. */
   508		rt2 = rt;
   509	
   510		rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
   511						   flowi4_to_flowi(fl4), NULL, 0);
   512		if (!IS_ERR(rt)) {
   513			if (rt != rt2)
   514				return rt;
   515		} else if (PTR_ERR(rt) == -EPERM) {
   516			rt = NULL;
   517		} else
   518			return rt;
   519	
 > 520		err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
   521		if (err)
   522			goto relookup_failed;
   523	
   524		if (inet_addr_type_dev_table(net, route_lookup_dev,
   525					     fl4_dec.saddr) == RTN_LOCAL) {
   526			rt2 = __ip_route_output_key(net, &fl4_dec);
   527			if (IS_ERR(rt2))
   528				err = PTR_ERR(rt2);
   529		} else {
   530			struct flowi4 fl4_2 = {};
   531			unsigned long orefdst;
   532	
   533			fl4_2.daddr = fl4_dec.saddr;
   534			rt2 = ip_route_output_key(net, &fl4_2);
   535			if (IS_ERR(rt2)) {
   536				err = PTR_ERR(rt2);
   537				goto relookup_failed;
   538			}
   539			/* Ugh! */
   540			orefdst = skb_in->_skb_refdst; /* save old refdst */
   541			skb_dst_set(skb_in, NULL);
   542			err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
   543					     RT_TOS(tos), rt2->dst.dev);
   544	
   545			dst_release(&rt2->dst);
   546			rt2 = skb_rtable(skb_in);
   547			skb_in->_skb_refdst = orefdst; /* restore old refdst */
   548		}
   549	
   550		if (err)
   551			goto relookup_failed;
   552	
   553		rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst,
   554						    flowi4_to_flowi(&fl4_dec), NULL,
   555						    XFRM_LOOKUP_ICMP);
   556		if (!IS_ERR(rt2)) {
   557			dst_release(&rt->dst);
   558			memcpy(fl4, &fl4_dec, sizeof(*fl4));
   559			rt = rt2;
   560		} else if (PTR_ERR(rt2) == -EPERM) {
   561			if (rt)
   562				dst_release(&rt->dst);
   563			return rt2;
   564		} else {
   565			err = PTR_ERR(rt2);
   566			goto relookup_failed;
   567		}
   568		return rt;
   569	
   570	relookup_failed:
   571		if (rt)
   572			return rt;
   573		return ERR_PTR(err);
   574	}
   575	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
  2023-09-29 22:42   ` kernel test robot
@ 2023-09-30  7:51     ` Florian Westphal
  0 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2023-09-30  7:51 UTC (permalink / raw)
  To: kernel test robot
  Cc: Florian Westphal, netdev, oe-kbuild-all, steffen.klassert,
	herbert, kernel test robot

kernel test robot <lkp@intel.com> wrote:
> Hi Florian,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on klassert-ipsec-next/master]
> [also build test WARNING on klassert-ipsec/master netfilter-nf/main linus/master v6.6-rc3 next-20230929]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/xfrm-pass-struct-net-to-xfrm_decode_session-wrappers/20230929-210047
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
> patch link:    https://lore.kernel.org/r/20230929125848.5445-2-fw%40strlen.de
> patch subject: [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers
> config: microblaze-defconfig (https://download.01.org/0day-ci/archive/20230930/202309300634.DBomJJ9W-lkp@intel.com/config)
> compiler: microblaze-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309300634.DBomJJ9W-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202309300634.DBomJJ9W-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    net/ipv4/icmp.c: In function 'icmp_route_lookup':
>    net/ipv4/icmp.c:520:43: error: passing argument 1 of 'xfrm_decode_session_reverse' from incompatible pointer type [-Werror=incompatible-pointer-types]
>      520 |         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
>          |                                           ^~~

I forgot to adjust the CONFIG_XFRM=n stub function, will ifx this up in v3.

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

end of thread, other threads:[~2023-09-30  7:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-29 12:58 [PATCH ipsec-next v2 0/3] xfrm: policy: replace session decode with flow dissector Florian Westphal
2023-09-29 12:58 ` [PATCH ipsec-next v2 1/3] xfrm: pass struct net to xfrm_decode_session wrappers Florian Westphal
2023-09-29 22:42   ` kernel test robot
2023-09-30  7:51     ` Florian Westphal
2023-09-29 22:52   ` kernel test robot
2023-09-30  2:00   ` kernel test robot
2023-09-29 12:58 ` [PATCH ipsec-next v2 2/3] xfrm: move mark and oif flowi decode into common code Florian Westphal
2023-09-29 12:58 ` [PATCH ipsec-next v2 3/3] xfrm: policy: replace session decode with flow dissector Florian Westphal

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