[PATCH RFC 0/5] pktgen IPsec support

[PATCH RFC 0/5] pktgen IPsec support

[Fan Du]

Hi, Dave/Steffen

Current pktgen IPsec supports only transport/ESP combinnation,
and it's buggy before I fixed transformed IP header/length issue
a few days ago.

This patchset enables user to do almost any IPsec transformation,
both transport/tunnel mode, and AH/ESP/IPcomp type.

Below configuration has been tested, and using Wireshark could decrypt
out plain text in good formation without any checksum/auth errors:

Mode/TYPE	AH	ESP
Transport	x	x
Tunnel		x	x

Fan Du (5):
  {pktgen, xfrm} Remove original pktgen ipsec fixed configuration
  {pktgen, xfrm} Using "pgset spi xxx" to spedifiy SA for a given flow
  {pktgen, xfrm} Construct skb dst for tunnel mode transformation
  {pktgen, xfrm} Introduce xfrm_state_lookup_byspi for pktgen
  {pktgen, xfrm} Correct xfrm state lock usage when transforming

 include/net/xfrm.h    |    7 ++-----
 net/core/pktgen.c     |   55 +++++++++++++++++++++++++++++++++----------------
 net/xfrm/xfrm_state.c |   43 +++++++++++++++-----------------------
 3 files changed, 56 insertions(+), 49 deletions(-)

-- 
1.7.9.5

[PATCH RFC 1/5] {pktgen, xfrm} Remove original pktgen ipsec fixed configuration

[Fan Du]

Cleanup original fixed IPsec output mode and encapuslation type,
As following patchset enable user to configure IPsec attribute.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/core/pktgen.c |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index a797fff..ab67986 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -387,8 +387,7 @@ struct pktgen_dev {
 	int node;               /* Memory node */
 
 #ifdef CONFIG_XFRM
-	__u8	ipsmode;		/* IPSEC mode (config) */
-	__u8	ipsproto;		/* IPSEC type (config) */
+
 #endif
 	char result[512];
 };
@@ -2482,10 +2481,7 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 
 	if (!x)
 		return 0;
-	/* XXX: we dont support tunnel mode for now until
-	 * we resolve the dst issue */
-	if (x->props.mode != XFRM_MODE_TRANSPORT)
-		return 0;
+
 
 	spin_lock(&x->lock);
 
@@ -3540,8 +3536,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 		goto out2;
 	}
 #ifdef CONFIG_XFRM
-	pkt_dev->ipsmode = XFRM_MODE_TRANSPORT;
-	pkt_dev->ipsproto = IPPROTO_ESP;
+
 #endif
 
 	return add_dev_to_thread(t, pkt_dev);
-- 
1.7.9.5

[PATCH RFC 2/5] {pktgen, xfrm} Using "pgset spi xxx" to spedifiy SA for a given flow

[Fan Du]

User could set specific SPI value to arm pktgen flow with IPsec
transformation, instead of looking up SA by sadr/daddr. The reaseon
to do so is because current state lookup scheme is both slow and, most
important of all, in fact pktgen doesn't need to match any SA state
addresses information, all it needs is the SA transfromation shell to
do the encapuslation.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/core/pktgen.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index ab67986..3e0a00f 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -387,6 +387,7 @@ struct pktgen_dev {
 	int node;               /* Memory node */
 
 #ifdef CONFIG_XFRM
+	__u32 spi;
 
 #endif
 	char result[512];
@@ -1475,6 +1476,16 @@ static ssize_t pktgen_if_write(struct file *file,
 		sprintf(pg_result, "OK: flows=%u", pkt_dev->cflows);
 		return count;
 	}
+	if (!strcmp(name, "spi")) {
+		len = num_arg(&user_buffer[i], 10, &value);
+		if (len < 0)
+			return len;
+
+		i += len;
+		pkt_dev->spi = value;
+		sprintf(pg_result, "OK: spi=%u", pkt_dev->spi);
+		return count;
+	}
 
 	if (!strcmp(name, "flowlen")) {
 		len = num_arg(&user_buffer[i], 10, &value);
-- 
1.7.9.5

[PATCH RFC 3/5] {pktgen,xfrm} Construct skb dst for tunnel mode transformation

[Fan Du]

IPsec tunnel mode encapuslation needs to set outter ip header
with right protocol/ttl/id value with regard to skb->dst->child.

Looking up a rt in a standard way is absolutely wrong for every
packet transmission. In a simple way, construct a dst by setting
neccessary information to make tunnel mode encapuslation working.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/core/pktgen.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 3e0a00f..c30df28 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -388,7 +388,8 @@ struct pktgen_dev {
 
 #ifdef CONFIG_XFRM
 	__u32 spi;
-
+	struct dst_entry dst, child;
+	struct dst_ops dstops;
 #endif
 	char result[512];
 };
@@ -2485,6 +2486,11 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 
 
 #ifdef CONFIG_XFRM
+u32 pktgen_dst_metrics[RTAX_MAX + 1] = {
+
+	[RTAX_HOPLIMIT] = 0x5, /* Set a static hoplimit */
+};
+
 static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 {
 	struct xfrm_state *x = pkt_dev->flows[pkt_dev->curfl].x;
@@ -2493,6 +2499,8 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 	if (!x)
 		return 0;
 
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		__skb_dst_set_noref(skb, &pkt_dev->dst, true);
 
 	spin_lock(&x->lock);
 
@@ -3547,7 +3555,16 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 		goto out2;
 	}
 #ifdef CONFIG_XFRM
-
+	/* xfrm tunnel mode needs additional dst to extract
+	 * outter ip header ttl/id field, here creat a phony one.
+	 * instead of looking for a valid rt, which definitely hurting
+	 * performance under such circumstance.
+	 */
+	pkt_dev->dstops.family = AF_INET;
+	pkt_dev->child.dev = pkt_dev->odev;
+	dst_init_metrics(&pkt_dev->child, pktgen_dst_metrics, false);
+	pkt_dev->dst.child = &pkt_dev->child;
+	pkt_dev->dst.ops = &pkt_dev->dstops;
 #endif
 
 	return add_dev_to_thread(t, pkt_dev);
-- 
1.7.9.5

[PATCH RFC 4/5] {pktgen, xfrm} Introduce xfrm_state_lookup_byspi for pktgen

[Fan Du]

Introduce xfrm_state_lookup_byspi to find user specified by custom
from "pgset spi xxx". Using this scheme, any flow regardless its
saddr/daddr could be transform by SA specified with configurable
spi.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 include/net/xfrm.h    |    7 ++-----
 net/core/pktgen.c     |   13 +++++--------
 net/xfrm/xfrm_state.c |   43 +++++++++++++++++--------------------------
 3 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 6b82fdf..68ad3f8 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1417,11 +1417,8 @@ struct xfrm_state *xfrm_state_find(const xfrm_address_t *daddr,
 				   struct xfrm_tmpl *tmpl,
 				   struct xfrm_policy *pol, int *err,
 				   unsigned short family);
-struct xfrm_state *xfrm_stateonly_find(struct net *net, u32 mark,
-				       xfrm_address_t *daddr,
-				       xfrm_address_t *saddr,
-				       unsigned short family,
-				       u8 mode, u8 proto, u32 reqid);
+struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi,
+					      unsigned short family);
 int xfrm_state_check_expire(struct xfrm_state *x);
 void xfrm_state_insert(struct xfrm_state *x);
 int xfrm_state_add(struct xfrm_state *x);
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index c30df28..6e30160 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2238,19 +2238,16 @@ static inline int f_pick(struct pktgen_dev *pkt_dev)
 /* If there was already an IPSEC SA, we keep it as is, else
  * we go look for it ...
 */
-#define DUMMY_MARK 0
 static void get_ipsec_sa(struct pktgen_dev *pkt_dev, int flow)
 {
 	struct xfrm_state *x = pkt_dev->flows[flow].x;
 	struct pktgen_net *pn = net_generic(dev_net(pkt_dev->odev), pg_net_id);
 	if (!x) {
-		/*slow path: we dont already have xfrm_state*/
-		x = xfrm_stateonly_find(pn->net, DUMMY_MARK,
-					(xfrm_address_t *)&pkt_dev->cur_daddr,
-					(xfrm_address_t *)&pkt_dev->cur_saddr,
-					AF_INET,
-					pkt_dev->ipsmode,
-					pkt_dev->ipsproto, 0);
+		/* We need as quick as possible to find the right SA
+		 * Searching with minimum criteria to archieve this.
+		 */
+		x = xfrm_state_lookup_byspi(pn->net, htonl(pkt_dev->spi),
+						AF_INET);
 		if (x) {
 			pkt_dev->flows[flow].x = x;
 			set_pkt_overhead(pkt_dev);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 68c2f35..6a9c402 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -892,38 +892,29 @@ out:
 	return x;
 }
 
-struct xfrm_state *
-xfrm_stateonly_find(struct net *net, u32 mark,
-		    xfrm_address_t *daddr, xfrm_address_t *saddr,
-		    unsigned short family, u8 mode, u8 proto, u32 reqid)
+struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi,
+					      unsigned short family)
 {
-	unsigned int h;
-	struct xfrm_state *rx = NULL, *x = NULL;
+	struct xfrm_state *x;
+	struct xfrm_state_walk *w;
 
-	spin_lock(&xfrm_state_lock);
-	h = xfrm_dst_hash(net, daddr, saddr, reqid, family);
-	hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
-		if (x->props.family == family &&
-		    x->props.reqid == reqid &&
-		    (mark & x->mark.m) == x->mark.v &&
-		    !(x->props.flags & XFRM_STATE_WILDRECV) &&
-		    xfrm_state_addr_check(x, daddr, saddr, family) &&
-		    mode == x->props.mode &&
-		    proto == x->id.proto &&
-		    x->km.state == XFRM_STATE_VALID) {
-			rx = x;
-			break;
-		}
-	}
+	spin_lock_bh(&xfrm_state_lock);
+	list_for_each_entry(w, &net->xfrm.state_all, all) {
 
-	if (rx)
-		xfrm_state_hold(rx);
-	spin_unlock(&xfrm_state_lock);
+		x = container_of(w, struct xfrm_state, km);
+		if (x->props.family != family ||
+			x->id.spi != spi)
+			continue;
 
+		spin_unlock_bh(&xfrm_state_lock);
+		xfrm_state_hold(x);
+		return x;
+	}
+	spin_unlock_bh(&xfrm_state_lock);
 
-	return rx;
+	return NULL;
 }
-EXPORT_SYMBOL(xfrm_stateonly_find);
+EXPORT_SYMBOL(xfrm_state_lookup_byspi);
 
 static void __xfrm_state_insert(struct xfrm_state *x)
 {
-- 
1.7.9.5

[PATCH RFC 5/5] {pktgen, xfrm} Correct xfrm state lock usage when transforming

[Fan Du]

xfrm_state lock protects its state, i.e., VALID/DEAD and statistics,
not the transforming procedure, as both mode/type output functions
are reentrant.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/core/pktgen.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 6e30160..6fbce38 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2499,8 +2499,6 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 	if (x->props.mode == XFRM_MODE_TUNNEL)
 		__skb_dst_set_noref(skb, &pkt_dev->dst, true);
 
-	spin_lock(&x->lock);
-
 	err = x->outer_mode->output(x, skb);
 	if (err)
 		goto error;
@@ -2508,10 +2506,11 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
 	if (err)
 		goto error;
 
+	spin_lock(&x->lock);
 	x->curlft.bytes += skb->len;
 	x->curlft.packets++;
-error:
 	spin_unlock(&x->lock);
+error:
 	return err;
 }
 
-- 
1.7.9.5

Re: [PATCH RFC 1/5] {pktgen, xfrm} Remove original pktgen ipsec fixed configuration

[Sergei Shtylyov]

Hello.

On 05-12-2013 11:57, Fan Du wrote:

> Cleanup original fixed IPsec output mode and encapuslation type,
> As following patchset enable user to configure IPsec attribute.

> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---
>   net/core/pktgen.c |   11 +++--------
>   1 file changed, 3 insertions(+), 8 deletions(-)

> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index a797fff..ab67986 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -387,8 +387,7 @@ struct pktgen_dev {
>   	int node;               /* Memory node */
>
>   #ifdef CONFIG_XFRM
> -	__u8	ipsmode;		/* IPSEC mode (config) */
> -	__u8	ipsproto;		/* IPSEC type (config) */
> +

    Why add this empty line which you'll remove in patch #3? And I'd also have 
removed empty #ifdef too...

>   #endif
>   	char result[512];
>   };
> @@ -2482,10 +2481,7 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
>
>   	if (!x)
>   		return 0;
> -	/* XXX: we dont support tunnel mode for now until
> -	 * we resolve the dst issue */
> -	if (x->props.mode != XFRM_MODE_TRANSPORT)
> -		return 0;
> +

    Why? There's enough empty lines already.

>
>   	spin_lock(&x->lock);
>
> @@ -3540,8 +3536,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
>   		goto out2;
>   	}
>   #ifdef CONFIG_XFRM
> -	pkt_dev->ipsmode = XFRM_MODE_TRANSPORT;
> -	pkt_dev->ipsproto = IPPROTO_ESP;
> +

    Same question about empty line removed in patch #3.

>   #endif

WBR, Sergei