netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* iproute2: make ip route list to search by metric too
@ 2017-11-17  0:40 Alexander Zubkov
  2017-11-18  1:44 ` Alexander Zubkov
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Zubkov @ 2017-11-17  0:40 UTC (permalink / raw)
  To: netdev

Hello all,

Currently routes in the Linux routing table have these "key" fields:
prefix, tos, table, metric (as I know). I.e. we cannot have two
different routes with the same set of this fields. And "ip route list"
command can be provided with all but one of those fields. We cannot
pass metric to it and this is inconvenient. I ask if this behaviour
can be changed by someone. We can even use "secondary" fields, for
example type, dev or via, but not metric unfortunately.
Sorry, I can not provide patches. I have written code long time ago. I
tried to trace it, but as I see it parses arguments and fills some
structures. And then my tries to understand failed.
I opened the bug: https://bugzilla.kernel.org/show_bug.cgi?id=197897,
but I was pointed out that this mailing list is a better place for
this question.

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

* Re: iproute2: make ip route list to search by metric too
  2017-11-17  0:40 iproute2: make ip route list to search by metric too Alexander Zubkov
@ 2017-11-18  1:44 ` Alexander Zubkov
  2017-11-18 10:11   ` Alexander Zubkov
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Zubkov @ 2017-11-18  1:44 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

Hello again,

Things turned out to be not so hard. Please take a look at the attached patch.
I'm only not sure if RTA_PRIORITY is enough. Because the print_route
function prints "metric" also for some situations with RTA_METRICS,
which I haven't managed to understand.

On Fri, Nov 17, 2017 at 1:40 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
> Hello all,
>
> Currently routes in the Linux routing table have these "key" fields:
> prefix, tos, table, metric (as I know). I.e. we cannot have two
> different routes with the same set of this fields. And "ip route list"
> command can be provided with all but one of those fields. We cannot
> pass metric to it and this is inconvenient. I ask if this behaviour
> can be changed by someone. We can even use "secondary" fields, for
> example type, dev or via, but not metric unfortunately.
> Sorry, I can not provide patches. I have written code long time ago. I
> tried to trace it, but as I see it parses arguments and fills some
> structures. And then my tries to understand failed.
> I opened the bug: https://bugzilla.kernel.org/show_bug.cgi?id=197897,
> but I was pointed out that this mailing list is a better place for
> this question.
>
> --
> Alexander Zubkov

[-- Attachment #2: list-metric.patch --]
[-- Type: text/x-patch, Size: 1253 bytes --]

--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -126,6 +126,8 @@ static struct
 	int oif, oifmask;
 	int mark, markmask;
 	int realm, realmmask;
+	int have_metric;
+	__u32 metric;
 	inet_prefix rprefsrc;
 	inet_prefix rvia;
 	inet_prefix rdst;
@@ -288,6 +290,14 @@ static int filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		if ((mark ^ filter.mark) & filter.markmask)
 			return 0;
 	}
+	if (filter.have_metric) {
+		__u32 metric = 0;
+
+		if (tb[RTA_PRIORITY])
+			metric = rta_getattr_u32(tb[RTA_PRIORITY]);
+		if (filter.metric != metric)
+			return 0;
+	}
 	if (filter.flushb &&
 	    r->rtm_family == AF_INET6 &&
 	    r->rtm_dst_len == 0 &&
@@ -1518,6 +1528,16 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 			if (get_unsigned(&mark, *argv, 0))
 				invarg("invalid mark value", *argv);
 			filter.markmask = -1;
+		} else if (matches(*argv, "metric") == 0 ||
+		           matches(*argv, "priority") == 0 ||
+		           strcmp(*argv, "preference") == 0) {
+			__u32 metric;
+
+			NEXT_ARG();
+			if (get_u32(&metric, *argv, 0))
+				invarg("\"metric\" value is invalid\n", *argv);
+			filter.metric = metric;
+			filter.have_metric = 1;
 		} else if (strcmp(*argv, "via") == 0) {
 			int family;
 

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

* Re: iproute2: make ip route list to search by metric too
  2017-11-18  1:44 ` Alexander Zubkov
@ 2017-11-18 10:11   ` Alexander Zubkov
  2017-12-04 19:39     ` Alexander Zubkov
  2017-12-17 12:02     ` [PATCH v3] iproute: list/flush/save filter also by metric Alexander Zubkov
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Zubkov @ 2017-11-18 10:11 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

I think this version will be better. It uses metric mask (like for
some other options) instead of simple yes/no flag.

On Sat, Nov 18, 2017 at 2:44 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
> Hello again,
>
> Things turned out to be not so hard. Please take a look at the attached patch.
> I'm only not sure if RTA_PRIORITY is enough. Because the print_route
> function prints "metric" also for some situations with RTA_METRICS,
> which I haven't managed to understand.
>
> On Fri, Nov 17, 2017 at 1:40 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
>> Hello all,
>>
>> Currently routes in the Linux routing table have these "key" fields:
>> prefix, tos, table, metric (as I know). I.e. we cannot have two
>> different routes with the same set of this fields. And "ip route list"
>> command can be provided with all but one of those fields. We cannot
>> pass metric to it and this is inconvenient. I ask if this behaviour
>> can be changed by someone. We can even use "secondary" fields, for
>> example type, dev or via, but not metric unfortunately.
>> Sorry, I can not provide patches. I have written code long time ago. I
>> tried to trace it, but as I see it parses arguments and fills some
>> structures. And then my tries to understand failed.
>> I opened the bug: https://bugzilla.kernel.org/show_bug.cgi?id=197897,
>> but I was pointed out that this mailing list is a better place for
>> this question.
>>
>> --
>> Alexander Zubkov

[-- Attachment #2: list-metric.patch.2 --]
[-- Type: application/octet-stream, Size: 1266 bytes --]

--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -126,6 +126,7 @@ static struct
 	int oif, oifmask;
 	int mark, markmask;
 	int realm, realmmask;
+	__u32 metric, metricmask;
 	inet_prefix rprefsrc;
 	inet_prefix rvia;
 	inet_prefix rdst;
@@ -288,6 +289,14 @@ static int filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		if ((mark ^ filter.mark) & filter.markmask)
 			return 0;
 	}
+	if (filter.metricmask) {
+		__u32 metric = 0;
+
+		if (tb[RTA_PRIORITY])
+			metric = rta_getattr_u32(tb[RTA_PRIORITY]);
+		if ((metric ^ filter.metric) & filter.metricmask)
+			return 0;
+	}
 	if (filter.flushb &&
 	    r->rtm_family == AF_INET6 &&
 	    r->rtm_dst_len == 0 &&
@@ -1518,6 +1527,16 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 			if (get_unsigned(&mark, *argv, 0))
 				invarg("invalid mark value", *argv);
 			filter.markmask = -1;
+		} else if (matches(*argv, "metric") == 0 ||
+		           matches(*argv, "priority") == 0 ||
+		           strcmp(*argv, "preference") == 0) {
+			__u32 metric;
+
+			NEXT_ARG();
+			if (get_u32(&metric, *argv, 0))
+				invarg("\"metric\" value is invalid\n", *argv);
+			filter.metric = metric;
+			filter.metricmask = -1;
 		} else if (strcmp(*argv, "via") == 0) {
 			int family;
 

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

* Re: iproute2: make ip route list to search by metric too
  2017-11-18 10:11   ` Alexander Zubkov
@ 2017-12-04 19:39     ` Alexander Zubkov
  2017-12-17 12:02     ` [PATCH v3] iproute: list/flush/save filter also by metric Alexander Zubkov
  1 sibling, 0 replies; 6+ messages in thread
From: Alexander Zubkov @ 2017-12-04 19:39 UTC (permalink / raw)
  To: netdev

Hello everybody,

Excuse me for probably being impatient. But may I have some feedback
on my proposal?

On Sat, Nov 18, 2017 at 11:11 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
> I think this version will be better. It uses metric mask (like for
> some other options) instead of simple yes/no flag.
>
> On Sat, Nov 18, 2017 at 2:44 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
>> Hello again,
>>
>> Things turned out to be not so hard. Please take a look at the attached patch.
>> I'm only not sure if RTA_PRIORITY is enough. Because the print_route
>> function prints "metric" also for some situations with RTA_METRICS,
>> which I haven't managed to understand.
>>
>> On Fri, Nov 17, 2017 at 1:40 AM, Alexander Zubkov <zubkov318@gmail.com> wrote:
>>> Hello all,
>>>
>>> Currently routes in the Linux routing table have these "key" fields:
>>> prefix, tos, table, metric (as I know). I.e. we cannot have two
>>> different routes with the same set of this fields. And "ip route list"
>>> command can be provided with all but one of those fields. We cannot
>>> pass metric to it and this is inconvenient. I ask if this behaviour
>>> can be changed by someone. We can even use "secondary" fields, for
>>> example type, dev or via, but not metric unfortunately.
>>> Sorry, I can not provide patches. I have written code long time ago. I
>>> tried to trace it, but as I see it parses arguments and fills some
>>> structures. And then my tries to understand failed.
>>> I opened the bug: https://bugzilla.kernel.org/show_bug.cgi?id=197897,
>>> but I was pointed out that this mailing list is a better place for
>>> this question.
>>>
>>> --
>>> Alexander Zubkov

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

* [PATCH v3] iproute: list/flush/save filter also by metric
  2017-11-18 10:11   ` Alexander Zubkov
  2017-12-04 19:39     ` Alexander Zubkov
@ 2017-12-17 12:02     ` Alexander Zubkov
  2017-12-19 16:22       ` Stephen Hemminger
  1 sibling, 1 reply; 6+ messages in thread
From: Alexander Zubkov @ 2017-12-17 12:02 UTC (permalink / raw)
  To: stephen; +Cc: zubkov318, netdev, Alexander Zubkov

Metric is one of the "unique key" fields of the route in Linux. But
still one can not use its value in filter while running ip list.
Because of this writing checks in scripts for example is incovenient.

Signed-off-by: Alexander Zubkov <green@msu.ru>
---
 ip/iproute.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 16eadab..32c93ed 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -126,6 +126,7 @@ static struct
 	int oif, oifmask;
 	int mark, markmask;
 	int realm, realmmask;
+	__u32 metric, metricmask;
 	inet_prefix rprefsrc;
 	inet_prefix rvia;
 	inet_prefix rdst;
@@ -288,6 +289,14 @@ static int filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		if ((mark ^ filter.mark) & filter.markmask)
 			return 0;
 	}
+	if (filter.metricmask) {
+		__u32 metric = 0;
+
+		if (tb[RTA_PRIORITY])
+			metric = rta_getattr_u32(tb[RTA_PRIORITY]);
+		if ((metric ^ filter.metric) & filter.metricmask)
+			return 0;
+	}
 	if (filter.flushb &&
 	    r->rtm_family == AF_INET6 &&
 	    r->rtm_dst_len == 0 &&
@@ -441,7 +450,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "src %s ",
 			rt_addr_n2a_rta(r->rtm_family, tb[RTA_PREFSRC]));
 	}
-	if (tb[RTA_PRIORITY])
+	if (tb[RTA_PRIORITY] && filter.metricmask != -1)
 		fprintf(fp, "metric %u ", rta_getattr_u32(tb[RTA_PRIORITY]));
 	if (r->rtm_flags & RTNH_F_DEAD)
 		fprintf(fp, "dead ");
@@ -1518,6 +1527,16 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 			if (get_unsigned(&mark, *argv, 0))
 				invarg("invalid mark value", *argv);
 			filter.markmask = -1;
+		} else if (matches(*argv, "metric") == 0 ||
+		           matches(*argv, "priority") == 0 ||
+		           strcmp(*argv, "preference") == 0) {
+			__u32 metric;
+
+			NEXT_ARG();
+			if (get_u32(&metric, *argv, 0))
+				invarg("\"metric\" value is invalid\n", *argv);
+			filter.metric = metric;
+			filter.metricmask = -1;
 		} else if (strcmp(*argv, "via") == 0) {
 			int family;
 

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

* Re: [PATCH v3] iproute: list/flush/save filter also by metric
  2017-12-17 12:02     ` [PATCH v3] iproute: list/flush/save filter also by metric Alexander Zubkov
@ 2017-12-19 16:22       ` Stephen Hemminger
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2017-12-19 16:22 UTC (permalink / raw)
  To: Alexander Zubkov; +Cc: zubkov318, netdev

On Sun, 17 Dec 2017 13:02:11 +0100
Alexander Zubkov <green@msu.ru> wrote:

> Metric is one of the "unique key" fields of the route in Linux. But
> still one can not use its value in filter while running ip list.
> Because of this writing checks in scripts for example is incovenient.
> 
> Signed-off-by: Alexander Zubkov <green@msu.ru>

Applied, thanks Alexander

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

end of thread, other threads:[~2017-12-19 16:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-17  0:40 iproute2: make ip route list to search by metric too Alexander Zubkov
2017-11-18  1:44 ` Alexander Zubkov
2017-11-18 10:11   ` Alexander Zubkov
2017-12-04 19:39     ` Alexander Zubkov
2017-12-17 12:02     ` [PATCH v3] iproute: list/flush/save filter also by metric Alexander Zubkov
2017-12-19 16:22       ` Stephen Hemminger

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