Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors
On Wed, 7 Apr 2021 16:55:41 -0600
David Ahern 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
Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors
On Wed, 7 Apr 2021 13:24:04 -0700
Jakub Kicinski 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
>
> > +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
Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors
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.
Re: [RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors
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
> +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.
[RFC net-next 1/1] seg6: add counters support for SRv6 Behaviors
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
---
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
