* [RFC net-next 0/1] seg6: Counters for SRv6 Behaviors @ 2021-04-07 18:03 Andrea Mayer 2021-04-07 18:03 ` [RFC net-next 1/1] seg6: add counters support " Andrea Mayer 2021-04-07 22:56 ` [RFC net-next 0/1] seg6: Counters " David Ahern 0 siblings, 2 replies; 7+ messages in thread From: Andrea Mayer @ 2021-04-07 18:03 UTC (permalink / raw) To: David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, linux-kernel, netdev Cc: Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam, Andrea Mayer This patch provides counters for SRv6 Behaviors as defined in [1], section 6. For each SRv6 Behavior instance, counters defined in [1] are: - the total number of packets that have been correctly processed; - the total amount of traffic in bytes of all packets that have been correctly processed; In addition, this patch introduces a new counter that counts the number of packets that have NOT been properly processed (i.e. errors) by an SRv6 Behavior instance. Counters are not only interesting for network monitoring purposes (i.e. counting the number of packets processed by a given behavior) but they also provide a simple tool for checking whether a behavior instance is working as we expect or not. Counters can be useful for troubleshooting misconfigured SRv6 networks. Indeed, an SRv6 Behavior can silently drop packets for very different reasons (i.e. wrong SID configuration, interfaces set with SID addresses, etc) without any notification/message to the user. Due to the nature of SRv6 networks, diagnostic tools such as ping and traceroute may be ineffective: paths used for reaching a given router can be totally different from the ones followed by probe packets. In addition, paths are often asymmetrical and this makes it even more difficult to keep up with the journey of the packets and to understand which behaviors are actually processing our traffic. When counters are enabled on an SRv6 Behavior instance, it is possible to verify if packets are actually processed by such behavior and what is the outcome of the processing. Therefore, the counters for SRv6 Behaviors offer an non-invasive observability point which can be leveraged for both traffic monitoring and troubleshooting purposes. [1] https://www.rfc-editor.org/rfc/rfc8986.html#name-counters Troubleshooting using SRv6 Behavior counters -------------------------------------------- Let's make a brief example to see how helpful counters can be for SRv6 networks. Let's consider a node where an SRv6 End Behavior receives an SRv6 packet whose Segment Left (SL) is equal to 0. In this case, the End Behavior (which accepts only packets with SL >= 1) discards the packet and increases the error counter. This information can be leveraged by the network operator for troubleshooting. Indeed, the error counter is telling the user that the packet: (i) arrived at the node; (ii) the packet has been taken into account by the SRv6 End behavior; (iii) but an error has occurred during the processing. The error (iii) could be caused by different reasons, such as wrong route settings on the node or due to an invalid SID List carried by the SRv6 packet. Anyway, the error counter is used to exclude that the packet did not arrive at the node or it has not been processed by the behavior at all. Turning on/off counters for SRv6 Behaviors ------------------------------------------ Each SRv6 Behavior instance can be configured, at the time of its creation, to make use of counters. This is done through iproute2 which allows the user to create an SRv6 Behavior instance specifying the optional "count" attribute as shown in the following example: $ ip -6 route add 2001:db8::1 encap seg6local action End count dev eth0 per-behavior counters can be shown by adding "-s" to the iproute2 command line, i.e.: $ ip -s -6 route show 2001:db8::1 2001:db8::1 encap seg6local action End packets 0 bytes 0 errors 0 dev eth0 #################################################### Impact of counters for SRv6 Behaviors on performance ==================================================== To determine the performance impact due to the introduction of counters in the SRv6 Behavior subsystem, we have carried out extensive tests. We chose to test the throughput achieved by the SRv6 End.DX2 Behavior because, among all the other behaviors implemented so far, it reaches the highest throughput which is around 1.5 Mpps (per core at 2.4 GHz on a Xeon(R) CPU E5-2630 v3) on kernel 5.12-rc2 using packets of size ~ 100 bytes. Three different tests were conducted in order to evaluate the overall throughput of the SRv6 End.DX2 Behavior in the following scenarios: 1) vanilla kernel (without the SRv6 Behavior counters patch) and a single instance of an SRv6 End.DX2 Behavior; 2) patched kernel with SRv6 Behavior counters and a single instance of an SRv6 End.DX2 Behavior with counters turned off; 3) patched kernel with SRv6 Behavior counters and a single instance of SRv6 End.DX2 Behavior with counters turned on. All tests were performed on a testbed deployed on the CloudLab facilities [2], a flexible infrastructure dedicated to scientific research on the future of Cloud Computing. Results of tests are shown in the following table: Scenario (1): average 1504764,81 pps (~1504,76 kpps); std. dev 3956,82 pps Scenario (2): average 1501469,78 pps (~1501,47 kpps); std. dev 2979,85 pps Scenario (3): average 1501315,13 pps (~1501,32 kpps); std. dev 2956,00 pps As can be observed, throughputs achieved in scenarios (2),(3) did not suffer any observable degradation compared to scenario (1). Comments, suggestions and improvements are very welcome! Thanks, Andrea [2] https://www.cloudlab.us Andrea Mayer (1): seg6: add counters support for SRv6 Behaviors include/uapi/linux/seg6_local.h | 8 ++ net/ipv6/seg6_local.c | 133 +++++++++++++++++++++++++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors 2021-04-07 18:03 [RFC net-next 0/1] seg6: Counters for SRv6 Behaviors Andrea Mayer @ 2021-04-07 18:03 ` Andrea Mayer 2021-04-07 20:24 ` Jakub Kicinski 2021-04-07 22:55 ` David Ahern 2021-04-07 22:56 ` [RFC net-next 0/1] seg6: Counters " David Ahern 1 sibling, 2 replies; 7+ messages in thread From: Andrea Mayer @ 2021-04-07 18:03 UTC (permalink / raw) To: David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, linux-kernel, netdev Cc: Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam, Andrea Mayer This patch provides counters for SRv6 Behaviors as defined in [1], section 6. For each SRv6 Behavior instance, the counters defined in [1] are: - the total number of packets that have been correctly processed; - the total amount of traffic in bytes of all packets that have been correctly processed; In addition, we introduces a new counter that counts the number of packets that have NOT been properly processed (i.e. errors) by an SRv6 Behavior instance. Each SRv6 Behavior instance can be configured, at the time of its creation, to make use of counters. This is done through iproute2 which allows the user to create an SRv6 Behavior instance specifying the optional "count" attribute as shown in the following example: $ ip -6 route add 2001:db8::1 encap seg6local action End count dev eth0 per-behavior counters can be shown by adding "-s" to the iproute2 command line, i.e.: $ ip -s -6 route show 2001:db8::1 2001:db8::1 encap seg6local action End packets 0 bytes 0 errors 0 dev eth0 [1] https://www.rfc-editor.org/rfc/rfc8986.html#name-counters Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it> --- include/uapi/linux/seg6_local.h | 8 ++ net/ipv6/seg6_local.c | 133 +++++++++++++++++++++++++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h index 3b39ef1dbb46..ae5e3fd12b73 100644 --- a/include/uapi/linux/seg6_local.h +++ b/include/uapi/linux/seg6_local.h @@ -27,6 +27,7 @@ enum { SEG6_LOCAL_OIF, SEG6_LOCAL_BPF, SEG6_LOCAL_VRFTABLE, + SEG6_LOCAL_COUNTERS, __SEG6_LOCAL_MAX, }; #define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1) @@ -78,4 +79,11 @@ enum { #define SEG6_LOCAL_BPF_PROG_MAX (__SEG6_LOCAL_BPF_PROG_MAX - 1) +/* SRv6 Behavior counters */ +struct seg6_local_counters { + __u64 rx_packets; + __u64 rx_bytes; + __u64 rx_errors; +}; + #endif diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c index 8936f48570fc..0f905a4410bd 100644 --- a/net/ipv6/seg6_local.c +++ b/net/ipv6/seg6_local.c @@ -93,6 +93,20 @@ struct seg6_end_dt_info { int hdrlen; }; +struct pcpu_seg6_local_counters { + u64_stats_t rx_packets; + u64_stats_t rx_bytes; + u64_stats_t rx_errors; + + struct u64_stats_sync syncp; +}; + +#define seg6_local_alloc_pcpu_counters(__gfp) \ + __netdev_alloc_pcpu_stats(struct pcpu_seg6_local_counters, \ + ((__gfp) | __GFP_ZERO)) + +#define SEG6_F_LOCAL_COUNTERS SEG6_F_ATTR(SEG6_LOCAL_COUNTERS) + struct seg6_local_lwt { int action; struct ipv6_sr_hdr *srh; @@ -105,6 +119,7 @@ struct seg6_local_lwt { #ifdef CONFIG_NET_L3_MASTER_DEV struct seg6_end_dt_info dt_info; #endif + struct pcpu_seg6_local_counters __percpu *pcpu_counters; int headroom; struct seg6_action_desc *desc; @@ -878,36 +893,43 @@ static struct seg6_action_desc seg6_action_table[] = { { .action = SEG6_LOCAL_ACTION_END, .attrs = 0, + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end, }, { .action = SEG6_LOCAL_ACTION_END_X, .attrs = SEG6_F_ATTR(SEG6_LOCAL_NH6), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_x, }, { .action = SEG6_LOCAL_ACTION_END_T, .attrs = SEG6_F_ATTR(SEG6_LOCAL_TABLE), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_t, }, { .action = SEG6_LOCAL_ACTION_END_DX2, .attrs = SEG6_F_ATTR(SEG6_LOCAL_OIF), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_dx2, }, { .action = SEG6_LOCAL_ACTION_END_DX6, .attrs = SEG6_F_ATTR(SEG6_LOCAL_NH6), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_dx6, }, { .action = SEG6_LOCAL_ACTION_END_DX4, .attrs = SEG6_F_ATTR(SEG6_LOCAL_NH4), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_dx4, }, { .action = SEG6_LOCAL_ACTION_END_DT4, .attrs = SEG6_F_ATTR(SEG6_LOCAL_VRFTABLE), + .optattrs = SEG6_F_LOCAL_COUNTERS, #ifdef CONFIG_NET_L3_MASTER_DEV .input = input_action_end_dt4, .slwt_ops = { @@ -919,30 +941,35 @@ static struct seg6_action_desc seg6_action_table[] = { .action = SEG6_LOCAL_ACTION_END_DT6, #ifdef CONFIG_NET_L3_MASTER_DEV .attrs = 0, - .optattrs = SEG6_F_ATTR(SEG6_LOCAL_TABLE) | + .optattrs = SEG6_F_LOCAL_COUNTERS | + SEG6_F_ATTR(SEG6_LOCAL_TABLE) | SEG6_F_ATTR(SEG6_LOCAL_VRFTABLE), .slwt_ops = { .build_state = seg6_end_dt6_build, }, #else .attrs = SEG6_F_ATTR(SEG6_LOCAL_TABLE), + .optattrs = SEG6_F_LOCAL_COUNTERS, #endif .input = input_action_end_dt6, }, { .action = SEG6_LOCAL_ACTION_END_B6, .attrs = SEG6_F_ATTR(SEG6_LOCAL_SRH), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_b6, }, { .action = SEG6_LOCAL_ACTION_END_B6_ENCAP, .attrs = SEG6_F_ATTR(SEG6_LOCAL_SRH), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_b6_encap, .static_headroom = sizeof(struct ipv6hdr), }, { .action = SEG6_LOCAL_ACTION_END_BPF, .attrs = SEG6_F_ATTR(SEG6_LOCAL_BPF), + .optattrs = SEG6_F_LOCAL_COUNTERS, .input = input_action_end_bpf, }, @@ -963,11 +990,36 @@ static struct seg6_action_desc *__get_action_desc(int action) return NULL; } +static bool seg6_lwtunnel_counters_enabled(struct seg6_local_lwt *slwt) +{ + return slwt->parsed_optattrs & SEG6_F_LOCAL_COUNTERS; +} + +static void seg6_local_update_rx_counters(struct seg6_local_lwt *slwt, + int len, int err) +{ + struct pcpu_seg6_local_counters *pcounters; + + pcounters = this_cpu_ptr(slwt->pcpu_counters); + u64_stats_update_begin(&pcounters->syncp); + + if (likely(!err)) { + u64_stats_inc(&pcounters->rx_packets); + u64_stats_add(&pcounters->rx_bytes, len); + } else { + u64_stats_inc(&pcounters->rx_errors); + } + + u64_stats_update_end(&pcounters->syncp); +} + static int seg6_local_input(struct sk_buff *skb) { struct dst_entry *orig_dst = skb_dst(skb); struct seg6_action_desc *desc; struct seg6_local_lwt *slwt; + int len = skb->len; + int rc; if (skb->protocol != htons(ETH_P_IPV6)) { kfree_skb(skb); @@ -977,7 +1029,14 @@ static int seg6_local_input(struct sk_buff *skb) slwt = seg6_local_lwtunnel(orig_dst->lwtstate); desc = slwt->desc; - return desc->input(skb, slwt); + rc = desc->input(skb, slwt); + + if (!seg6_lwtunnel_counters_enabled(slwt)) + return rc; + + seg6_local_update_rx_counters(slwt, len, rc); + + return rc; } static const struct nla_policy seg6_local_policy[SEG6_LOCAL_MAX + 1] = { @@ -992,6 +1051,8 @@ static const struct nla_policy seg6_local_policy[SEG6_LOCAL_MAX + 1] = { [SEG6_LOCAL_IIF] = { .type = NLA_U32 }, [SEG6_LOCAL_OIF] = { .type = NLA_U32 }, [SEG6_LOCAL_BPF] = { .type = NLA_NESTED }, + [SEG6_LOCAL_COUNTERS] = { .type = NLA_BINARY, + .len = sizeof(struct seg6_local_counters) }, }; static int parse_nla_srh(struct nlattr **attrs, struct seg6_local_lwt *slwt) @@ -1296,6 +1357,67 @@ static void destroy_attr_bpf(struct seg6_local_lwt *slwt) bpf_prog_put(slwt->bpf.prog); } +static int parse_nla_counters(struct nlattr **attrs, + struct seg6_local_lwt *slwt) +{ + struct pcpu_seg6_local_counters __percpu *pcounters; + + pcounters = seg6_local_alloc_pcpu_counters(GFP_KERNEL); + if (!pcounters) + return -ENOMEM; + + slwt->pcpu_counters = pcounters; + + return 0; +} + +static int put_nla_counters(struct sk_buff *skb, struct seg6_local_lwt *slwt) +{ + struct seg6_local_counters counters = { 0, 0, 0 }; + struct nlattr *nla; + int i; + + nla = nla_reserve(skb, SEG6_LOCAL_COUNTERS, sizeof(counters)); + if (!nla) + return -EMSGSIZE; + + for_each_possible_cpu(i) { + struct pcpu_seg6_local_counters *pcounters; + u64 rx_packets, rx_bytes, rx_errors; + unsigned int start; + + pcounters = per_cpu_ptr(slwt->pcpu_counters, i); + do { + start = u64_stats_fetch_begin_irq(&pcounters->syncp); + + rx_packets = u64_stats_read(&pcounters->rx_packets); + rx_bytes = u64_stats_read(&pcounters->rx_bytes); + rx_errors = u64_stats_read(&pcounters->rx_errors); + + } while (u64_stats_fetch_retry_irq(&pcounters->syncp, start)); + + counters.rx_packets += rx_packets; + counters.rx_bytes += rx_bytes; + counters.rx_errors += rx_errors; + } + + memcpy(nla_data(nla), &counters, sizeof(counters)); + + return 0; +} + +static int cmp_nla_counters(struct seg6_local_lwt *a, struct seg6_local_lwt *b) +{ + /* a and b are equals if both have pcpu_counters set or not */ + return (!!((unsigned long)a->pcpu_counters)) ^ + (!!((unsigned long)b->pcpu_counters)); +} + +static void destroy_attr_counters(struct seg6_local_lwt *slwt) +{ + free_percpu(slwt->pcpu_counters); +} + struct seg6_action_param { int (*parse)(struct nlattr **attrs, struct seg6_local_lwt *slwt); int (*put)(struct sk_buff *skb, struct seg6_local_lwt *slwt); @@ -1343,6 +1465,10 @@ static struct seg6_action_param seg6_action_params[SEG6_LOCAL_MAX + 1] = { .put = put_nla_vrftable, .cmp = cmp_nla_vrftable }, + [SEG6_LOCAL_COUNTERS] = { .parse = parse_nla_counters, + .put = put_nla_counters, + .cmp = cmp_nla_counters, + .destroy = destroy_attr_counters }, }; /* call the destroy() callback (if available) for each set attribute in @@ -1645,6 +1771,9 @@ static int seg6_local_get_encap_size(struct lwtunnel_state *lwt) if (attrs & SEG6_F_ATTR(SEG6_LOCAL_VRFTABLE)) nlsize += nla_total_size(4); + if (attrs & SEG6_F_LOCAL_COUNTERS) + nlsize += nla_total_size(sizeof(struct seg6_local_counters)); + return nlsize; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors 2021-04-07 18:03 ` [RFC net-next 1/1] seg6: add counters support " Andrea Mayer @ 2021-04-07 20:24 ` Jakub Kicinski 2021-04-09 12:43 ` Andrea Mayer 2021-04-07 22:55 ` David Ahern 1 sibling, 1 reply; 7+ messages in thread From: Jakub Kicinski @ 2021-04-07 20:24 UTC (permalink / raw) To: Andrea Mayer Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, linux-kernel, netdev, Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam On Wed, 7 Apr 2021 20:03:32 +0200 Andrea Mayer wrote: > This patch provides counters for SRv6 Behaviors as defined in [1], section > 6. For each SRv6 Behavior instance, the counters defined in [1] are: > > - the total number of packets that have been correctly processed; > - the total amount of traffic in bytes of all packets that have been > correctly processed; > > In addition, we introduces a new counter that counts the number of packets > that have NOT been properly processed (i.e. errors) by an SRv6 Behavior > instance. > > Each SRv6 Behavior instance can be configured, at the time of its creation, > to make use of counters. > This is done through iproute2 which allows the user to create an SRv6 > Behavior instance specifying the optional "count" attribute as shown in the > following example: > > $ ip -6 route add 2001:db8::1 encap seg6local action End count dev eth0 > > per-behavior counters can be shown by adding "-s" to the iproute2 command > line, i.e.: > > $ ip -s -6 route show 2001:db8::1 > 2001:db8::1 encap seg6local action End packets 0 bytes 0 errors 0 dev eth0 > > [1] https://www.rfc-editor.org/rfc/rfc8986.html#name-counters > > Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it> > +static int put_nla_counters(struct sk_buff *skb, struct seg6_local_lwt *slwt) > +{ > + struct seg6_local_counters counters = { 0, 0, 0 }; > + struct nlattr *nla; > + int i; > + > + nla = nla_reserve(skb, SEG6_LOCAL_COUNTERS, sizeof(counters)); > + if (!nla) > + return -EMSGSIZE; nla_reserve_64bit(), IIUC netlink guarantees alignment of 64 bit values. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors 2021-04-07 20:24 ` Jakub Kicinski @ 2021-04-09 12:43 ` Andrea Mayer 0 siblings, 0 replies; 7+ messages in thread From: Andrea Mayer @ 2021-04-09 12:43 UTC (permalink / raw) To: Jakub Kicinski Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, linux-kernel, netdev, Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam, Andrea Mayer On Wed, 7 Apr 2021 13:24:04 -0700 Jakub Kicinski <kuba@kernel.org> wrote: > On Wed, 7 Apr 2021 20:03:32 +0200 Andrea Mayer wrote: > > This patch provides counters for SRv6 Behaviors as defined in [1], section > > 6. For each SRv6 Behavior instance, the counters defined in [1] are: > > > > - the total number of packets that have been correctly processed; > > - the total amount of traffic in bytes of all packets that have been > > correctly processed; > > > > In addition, we introduces a new counter that counts the number of packets > > that have NOT been properly processed (i.e. errors) by an SRv6 Behavior > > instance. > > > > Each SRv6 Behavior instance can be configured, at the time of its creation, > > to make use of counters. > > This is done through iproute2 which allows the user to create an SRv6 > > Behavior instance specifying the optional "count" attribute as shown in the > > following example: > > > > $ ip -6 route add 2001:db8::1 encap seg6local action End count dev eth0 > > > > per-behavior counters can be shown by adding "-s" to the iproute2 command > > line, i.e.: > > > > $ ip -s -6 route show 2001:db8::1 > > 2001:db8::1 encap seg6local action End packets 0 bytes 0 errors 0 dev eth0 > > > > [1] https://www.rfc-editor.org/rfc/rfc8986.html#name-counters > > > > Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it> > > > +static int put_nla_counters(struct sk_buff *skb, struct seg6_local_lwt *slwt) > > +{ > > + struct seg6_local_counters counters = { 0, 0, 0 }; > > + struct nlattr *nla; > > + int i; > > + > > + nla = nla_reserve(skb, SEG6_LOCAL_COUNTERS, sizeof(counters)); > > + if (!nla) > > + return -EMSGSIZE; > > nla_reserve_64bit(), IIUC netlink guarantees alignment of 64 bit values. Hi Jakub, thanks for your review! Yes, we should guarantee alignment of 64 bit values. I will definitely follow your advice. Andrea ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors 2021-04-07 18:03 ` [RFC net-next 1/1] seg6: add counters support " Andrea Mayer 2021-04-07 20:24 ` Jakub Kicinski @ 2021-04-07 22:55 ` David Ahern 2021-04-09 17:55 ` Andrea Mayer 1 sibling, 1 reply; 7+ messages in thread From: David Ahern @ 2021-04-07 22:55 UTC (permalink / raw) To: Andrea Mayer, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, linux-kernel, netdev Cc: Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam On 4/7/21 12:03 PM, Andrea Mayer wrote: > diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h > index 3b39ef1dbb46..ae5e3fd12b73 100644 > --- a/include/uapi/linux/seg6_local.h > +++ b/include/uapi/linux/seg6_local.h > @@ -27,6 +27,7 @@ enum { > SEG6_LOCAL_OIF, > SEG6_LOCAL_BPF, > SEG6_LOCAL_VRFTABLE, > + SEG6_LOCAL_COUNTERS, > __SEG6_LOCAL_MAX, > }; > #define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1) > @@ -78,4 +79,11 @@ enum { > > #define SEG6_LOCAL_BPF_PROG_MAX (__SEG6_LOCAL_BPF_PROG_MAX - 1) > > +/* SRv6 Behavior counters */ > +struct seg6_local_counters { > + __u64 rx_packets; > + __u64 rx_bytes; > + __u64 rx_errors; > +}; > + > #endif It's highly likely that more stats would get added over time. It would be good to document that here for interested parties and then make sure iproute2 can handle different sized stats structs. e.g., commit support to your repo, then add a new one (e.g, rx_drops) and verify the combinations handle it. e.g., old kernel - new iproute2, new kernel - old iproute, old - old and new-new. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors 2021-04-07 22:55 ` David Ahern @ 2021-04-09 17:55 ` Andrea Mayer 0 siblings, 0 replies; 7+ messages in thread From: Andrea Mayer @ 2021-04-09 17:55 UTC (permalink / raw) To: David Ahern Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, linux-kernel, netdev, Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam, Andrea Mayer On Wed, 7 Apr 2021 16:55:41 -0600 David Ahern <dsahern@gmail.com> wrote: > On 4/7/21 12:03 PM, Andrea Mayer wrote: > > diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h > > index 3b39ef1dbb46..ae5e3fd12b73 100644 > > --- a/include/uapi/linux/seg6_local.h > > +++ b/include/uapi/linux/seg6_local.h > > @@ -27,6 +27,7 @@ enum { > > SEG6_LOCAL_OIF, > > SEG6_LOCAL_BPF, > > SEG6_LOCAL_VRFTABLE, > > + SEG6_LOCAL_COUNTERS, > > __SEG6_LOCAL_MAX, > > }; > > #define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1) > > @@ -78,4 +79,11 @@ enum { > > > > #define SEG6_LOCAL_BPF_PROG_MAX (__SEG6_LOCAL_BPF_PROG_MAX - 1) > > > > +/* SRv6 Behavior counters */ > > +struct seg6_local_counters { > > + __u64 rx_packets; > > + __u64 rx_bytes; > > + __u64 rx_errors; > > +}; > > + > > #endif > > It's highly likely that more stats would get added over time. It would > be good to document that here for interested parties and then make sure > iproute2 can handle different sized stats structs. e.g., commit support > to your repo, then add a new one (e.g, rx_drops) and verify the > combinations handle it. e.g., old kernel - new iproute2, new kernel - > old iproute, old - old and new-new. > Hi David, thanks for your review. I totally agree with you: we may want to add other counters in the future, even if they are not considered in RFC8986. With that in mind, the shared struct seg6_local_counters is not the best way to go if we want to add other counters (because it will be difficult to manage different sized structures when considering different kernel/iproute2 versions). To make it easier adding new counters, instead of sharing the struct seg6_local_counters, I would use netlink nested attributes to exchange counters individually. In this way, only recognized (nested) attributes can be processed by both the kernel and iproute2. For example: enum { SEG6_LOCAL_CNT_UNSPEC, SEG6_LOCAL_CNT_PAD, /* padding for 64 bits values */ SEG6_LOCAL_CNT_RX_PACKETS, SEG6_LOCAL_CNT_RX_BYTES, SEG6_LOCAL_CNT_RX_ERRORS, __SEG6_LOCAL_CNT_MAX, }; #define SEG6_LOCAL_CNT_MAX (__SEG6_LOCAL_CNT_MAX - 1) updating the policy for SEG6_LOCAL_COUNTERS to NLA_NESTED. Then, I create a new policy for counters which handles each supported counter separately. static const struct nla_policy seg6_local_counters_policy[SEG6_LOCAL_CNT_MAX + 1] = { [SEG6_LOCAL_CNT_RX_PACKETS] = { .type = NLA_U64 }, [SEG6_LOCAL_CNT_RX_BYTES] = { .type = NLA_U64 }, [SEG6_LOCAL_CNT_RX_ERRORS] = { .type = NLA_U64 }, }; At the end, I update the parse_nla_counters(), put_nla_counters(), etc according to the changes, i.e: - nla_parse_nested() in parse_nla_counters(); - nla_nest_{start/end}() and for each supported counter nla_put_u64_64bit() in put_nla_counters(). On the iproute2 side, we have to update the code to reflect the changes discussed above. I plan to issue an RFC v2 in a few days. Andrea ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC net-next 0/1] seg6: Counters for SRv6 Behaviors 2021-04-07 18:03 [RFC net-next 0/1] seg6: Counters for SRv6 Behaviors Andrea Mayer 2021-04-07 18:03 ` [RFC net-next 1/1] seg6: add counters support " Andrea Mayer @ 2021-04-07 22:56 ` David Ahern 1 sibling, 0 replies; 7+ messages in thread From: David Ahern @ 2021-04-07 22:56 UTC (permalink / raw) To: Andrea Mayer, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, linux-kernel, netdev Cc: Stefano Salsano, Paolo Lungaroni, Ahmed Abdelsalam Since this is a single patch set, just put this good cover letter content as the message in the patch. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-09 17:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-07 18:03 [RFC net-next 0/1] seg6: Counters for SRv6 Behaviors Andrea Mayer 2021-04-07 18:03 ` [RFC net-next 1/1] seg6: add counters support " Andrea Mayer 2021-04-07 20:24 ` Jakub Kicinski 2021-04-09 12:43 ` Andrea Mayer 2021-04-07 22:55 ` David Ahern 2021-04-09 17:55 ` Andrea Mayer 2021-04-07 22:56 ` [RFC net-next 0/1] seg6: Counters " David Ahern
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).