[PATCH iproute2 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[PATCH iproute2 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[Thomas Egerer]

Using 'ip deleteall' with policies that have marks, fails unless you
eplicitely specify the mark values. This is very uncomfortable when
bulk-deleting policies and states. With this patch all relevant states
and policies are wiped by 'ip deleteall' regardless of their mark
values.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 ip/xfrm_policy.c |  9 +++++++++
 ip/xfrm_state.c  | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index e2fa771..d544026 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -753,6 +753,15 @@ static int xfrm_policy_keep(const struct sockaddr_nl *who,
 	xpid->dir = xpinfo->dir;
 	xpid->index = xpinfo->index;
 
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+				(void *)RTA_DATA(tb[XFRMA_MARK]), tb[XFRMA_MARK]->rta_len);
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 3e20d6e..85d959c 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -1081,6 +1081,7 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	struct nlmsghdr *new_n;
 	struct xfrm_usersa_id *xsid;
+	struct rtattr *tb[XFRMA_MAX+1];
 
 	if (n->nlmsg_type != XFRM_MSG_NEWSA) {
 		fprintf(stderr, "Not a state: %08x %08x %08x\n",
@@ -1117,6 +1118,17 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	addattr_l(new_n, xb->size, XFRMA_SRCADDR, &xsinfo->saddr,
 		  sizeof(xsid->daddr));
 
+	parse_rtattr(tb, XFRMA_MAX, XFRMS_RTA(xsinfo), len);
+
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+				(void *)RTA_DATA(tb[XFRMA_MARK]), tb[XFRMA_MARK]->rta_len);
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
-- 
2.6.4

Re: [PATCH iproute2 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[Stephen Hemminger]

On Mon, 30 Oct 2017 19:11:46 +0100
Thomas Egerer <thomas.egerer@secunet.com> wrote:

>  
> +	if (tb[XFRMA_MARK]) {
> +		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
> +				(void *)RTA_DATA(tb[XFRMA_MARK]), tb[XFRMA_MARK]->rta_len);

Since addattr_l already uses const void * for the attribute argument,
the cast here is unnecessary. Also try and break long lines if possible.

Also, you should probably be using RTA_PAYLOAD() rather than dereferncing rta_len
here.

[PATCH iproute2 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[Thomas Egerer]

Using 'ip deleteall' with policies that have marks, fails unless you
eplicitely specify the mark values. This is very uncomfortable when
bulk-deleting policies and states. With this patch all relevant states
and policies are wiped by 'ip deleteall' regardless of their mark
values.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 ip/xfrm_policy.c | 10 ++++++++++
 ip/xfrm_state.c  | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index e2fa771..feea7d6 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -753,6 +753,16 @@ static int xfrm_policy_keep(const struct sockaddr_nl *who,
 	xpid->dir = xpinfo->dir;
 	xpid->index = xpinfo->index;
 
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+				  RTA_DATA(tb[XFRMA_MARK]),
+				  RTA_PAYLOAD(tb[XFRMA_MARK]));
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 3e20d6e..e193623 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -1081,6 +1081,7 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	struct nlmsghdr *new_n;
 	struct xfrm_usersa_id *xsid;
+	struct rtattr *tb[XFRMA_MAX+1];
 
 	if (n->nlmsg_type != XFRM_MSG_NEWSA) {
 		fprintf(stderr, "Not a state: %08x %08x %08x\n",
@@ -1117,6 +1118,18 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	addattr_l(new_n, xb->size, XFRMA_SRCADDR, &xsinfo->saddr,
 		  sizeof(xsid->daddr));
 
+	parse_rtattr(tb, XFRMA_MAX, XFRMS_RTA(xsinfo), len);
+
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+						  RTA_DATA(tb[XFRMA_MARK]),
+						  RTA_PAYLOAD(tb[XFRMA_MARK]));
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
-- 
2.6.4

Re: [PATCH iproute2 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[Stephen Hemminger]

On Wed, 1 Nov 2017 10:52:54 +0100
Thomas Egerer <thomas.egerer@secunet.com> wrote:

> Using 'ip deleteall' with policies that have marks, fails unless you
> eplicitely specify the mark values. This is very uncomfortable when
> bulk-deleting policies and states. With this patch all relevant states
> and policies are wiped by 'ip deleteall' regardless of their mark
> values.
> 
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>

Like netdev to the kernel, you need to resend the whole patch series.

[PATCH iproute2-resend 0/3] *** SUBJECT HERE ***

[Thomas Egerer]

Hello *,

the following set of three patches tries deals with socket policies.
The first patch adresses the missing filter option for socket
polices. Especially when dealing with many of those, it is quite
cumbersome to filter them from the iproute2-output. So an option
to remove them from the output has been added.
Also when trying to deleteall policies iproute2 tries to delete
socket based policies, too. The result is an error message which
is misleading and unnecessary. So the second patch skips all
socket policies when deleteall-ing policies.
The third patch allow to deleteall policies and states even if
they have a mark. I'm not sure if the current behavior is
intended but if iproute2 finds a policy or state with a mark
it tries to delete the corresponding policy/state *without*
a mark. Also the result is an error and the policy/state is
not deleted.
Resend with modifications as requested by Stephen.

Regards
Thomas

Thomas Egerer (3):
  xfrm_policy: Add filter option for socket policies
  xfrm_policy: Do not attempt to deleteall a socket policy
  xfrm_{state,policy}: Allow to deleteall polices/states with marks

 ip/xfrm.h        |  1 +
 ip/xfrm_policy.c | 22 +++++++++++++++++++++-
 ip/xfrm_state.c  | 13 +++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.6.4

[PATCH iproute2-resend 1/3] xfrm_policy: Add filter option for socket policies

[Thomas Egerer]

Listing policies on systems with a lot of socket policies can be
confusing due to the number of returned polices. Even if socket polices
are not of interest, they cannot be filtered. This patch adds an option
to filter all socket policies from the output.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 ip/xfrm.h        | 1 +
 ip/xfrm_policy.c | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ip/xfrm.h b/ip/xfrm.h
index 54d80ce..8566d63 100644
--- a/ip/xfrm.h
+++ b/ip/xfrm.h
@@ -90,6 +90,7 @@ struct xfrm_filter {
 	__u8 action_mask;
 	__u32 priority_mask;
 	__u8 policy_flags_mask;
+	__u8 filter_socket;
 
 	__u8 ptype;
 	__u8 ptype_mask;
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 98460a0..7d2139e 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -58,7 +58,7 @@ static void usage(void)
 	fprintf(stderr, "        [ LIMIT-LIST ] [ TMPL-LIST ]\n");
 	fprintf(stderr, "Usage: ip xfrm policy { delete | get } { SELECTOR | index INDEX } dir DIR\n");
 	fprintf(stderr, "        [ ctx CTX ] [ mark MARK [ mask MASK ] ] [ ptype PTYPE ]\n");
-	fprintf(stderr, "Usage: ip xfrm policy { deleteall | list } [ SELECTOR ] [ dir DIR ]\n");
+	fprintf(stderr, "Usage: ip xfrm policy { deleteall | list } [ nosock ] [ SELECTOR ] [ dir DIR ]\n");
 	fprintf(stderr, "        [ index INDEX ] [ ptype PTYPE ] [ action ACTION ] [ priority PRIORITY ]\n");
 	fprintf(stderr, "        [ flag FLAG-LIST ]\n");
 	fprintf(stderr, "Usage: ip xfrm policy flush [ ptype PTYPE ]\n");
@@ -403,6 +403,9 @@ static int xfrm_policy_filter_match(struct xfrm_userpolicy_info *xpinfo,
 	if ((xpinfo->dir^filter.xpinfo.dir)&filter.dir_mask)
 		return 0;
 
+	if (filter.filter_socket && (xpinfo->dir >= XFRM_POLICY_MAX))
+		return 0;
+
 	if ((ptype^filter.ptype)&filter.ptype_mask)
 		return 0;
 
@@ -806,6 +809,9 @@ static int xfrm_policy_list_or_deleteall(int argc, char **argv, int deleteall)
 
 			filter.policy_flags_mask = XFRM_FILTER_MASK_FULL;
 
+		} else if (strcmp(*argv, "nosock") == 0) {
+			/* filter all socket-based policies */
+			filter.filter_socket = 1;
 		} else {
 			if (selp)
 				invarg("unknown", *argv);
-- 
2.6.4

[PATCH iproute2-resend 2/3] xfrm_policy: Do not attempt to deleteall a socket policy

[Thomas Egerer]

Socket polices are added to a socket using setsockopt(2). They cannot be
deleted by iproute2. The attempt to delete them causes an error
(EINVAL).
To avoid this unnecessary error message all socket policies are skipped
in xfrm_policy_keep.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 ip/xfrm_policy.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 7d2139e..e2fa771 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -735,6 +735,10 @@ static int xfrm_policy_keep(const struct sockaddr_nl *who,
 	if (!xfrm_policy_filter_match(xpinfo, ptype))
 		return 0;
 
+	/* can't delete socket policies */
+	if (xpinfo->dir >= XFRM_POLICY_MAX)
+		return 0;
+
 	if (xb->offset + NLMSG_LENGTH(sizeof(*xpid)) > xb->size)
 		return 0;
 
-- 
2.6.4

[PATCH iproute2-resend 3/3] xfrm_{state,policy}: Allow to deleteall polices/states with marks

[Thomas Egerer]

Using 'ip deleteall' with policies that have marks, fails unless you
eplicitely specify the mark values. This is very uncomfortable when
bulk-deleting policies and states. With this patch all relevant states
and policies are wiped by 'ip deleteall' regardless of their mark
values.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 ip/xfrm_policy.c | 10 ++++++++++
 ip/xfrm_state.c  | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index e2fa771..feea7d6 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -753,6 +753,16 @@ static int xfrm_policy_keep(const struct sockaddr_nl *who,
 	xpid->dir = xpinfo->dir;
 	xpid->index = xpinfo->index;
 
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+				  RTA_DATA(tb[XFRMA_MARK]),
+				  RTA_PAYLOAD(tb[XFRMA_MARK]));
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 3e20d6e..e193623 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -1081,6 +1081,7 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	struct nlmsghdr *new_n;
 	struct xfrm_usersa_id *xsid;
+	struct rtattr *tb[XFRMA_MAX+1];
 
 	if (n->nlmsg_type != XFRM_MSG_NEWSA) {
 		fprintf(stderr, "Not a state: %08x %08x %08x\n",
@@ -1117,6 +1118,18 @@ static int xfrm_state_keep(const struct sockaddr_nl *who,
 	addattr_l(new_n, xb->size, XFRMA_SRCADDR, &xsinfo->saddr,
 		  sizeof(xsid->daddr));
 
+	parse_rtattr(tb, XFRMA_MAX, XFRMS_RTA(xsinfo), len);
+
+	if (tb[XFRMA_MARK]) {
+		int r = addattr_l(new_n, xb->size, XFRMA_MARK,
+						  RTA_DATA(tb[XFRMA_MARK]),
+						  RTA_PAYLOAD(tb[XFRMA_MARK]));
+		if (r < 0) {
+			fprintf(stderr, "%s: XFRMA_MARK failed\n", __func__);
+			exit(1);
+		}
+	}
+
 	xb->offset += new_n->nlmsg_len;
 	xb->nlmsg_count++;
 
-- 
2.6.4

Re: [PATCH iproute2-resend 0/3] *** SUBJECT HERE ***

[Stephen Hemminger]

On Thu, 2 Nov 2017 14:13:22 +0100
Thomas Egerer <thomas.egerer@secunet.com> wrote:

> Hello *,
> 
> the following set of three patches tries deals with socket policies.
> The first patch adresses the missing filter option for socket
> polices. Especially when dealing with many of those, it is quite
> cumbersome to filter them from the iproute2-output. So an option
> to remove them from the output has been added.
> Also when trying to deleteall policies iproute2 tries to delete
> socket based policies, too. The result is an error message which
> is misleading and unnecessary. So the second patch skips all
> socket policies when deleteall-ing policies.
> The third patch allow to deleteall policies and states even if
> they have a mark. I'm not sure if the current behavior is
> intended but if iproute2 finds a policy or state with a mark
> it tries to delete the corresponding policy/state *without*
> a mark. Also the result is an error and the policy/state is
> not deleted.
> Resend with modifications as requested by Stephen.
> 
> Regards
> Thomas
> 
> Thomas Egerer (3):
>   xfrm_policy: Add filter option for socket policies
>   xfrm_policy: Do not attempt to deleteall a socket policy
>   xfrm_{state,policy}: Allow to deleteall polices/states with marks
> 
>  ip/xfrm.h        |  1 +
>  ip/xfrm_policy.c | 22 +++++++++++++++++++++-
>  ip/xfrm_state.c  | 13 +++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 

Applied.