Re: [ovs-dev] [PATCH V10 05/33] tc: Add tc flower functions

[Flavio Leitner]

On Mon, Jun 12, 2017 at 05:57:37PM +0300, Roi Dayan wrote:
> 
> 
> On 09/06/2017 21:37, Flavio Leitner wrote:
> > On Thu, Jun 08, 2017 at 02:46:22PM +0300, Roi Dayan wrote:
> > > Add tc helper functions to query and manipulate the flower classifier.
> > > 
> > > Signed-off-by: Paul Blakey 
> > > Co-authored-by: Roi Dayan 
> > > Signed-off-by: Roi Dayan 
> > 
> > Acked-by: Flavio Leitner 
> > 
> > Not sure why SCTP wasn't implemented, but not a blocker either.
> 
> 
> Hi Flavio,
> I didn't add it because later needed to spread changes across more
> commits to support it to the end. planned to do it in a later
> commit after the series.
> All the other changes took me long enough and I wanted to minimize the
> wait.

OK, it seems that at this point we could merge and let others start to
contribute as well.

fbl


> 
> > 
> > > --- a/lib/tc.c
> > > +++ b/lib/tc.c
> > [...]
> > > +
> > > +#define JIFFIES_TO_MS(x) (x * 10)
> > > +};
> > 
> > Thanks for fixing this
> > 
> > > +static void
> > > +nl_parse_tcf(const struct tcf_t *tm, struct tc_flower *flower)
> > > +{
> > > +flower->lastused = time_msec() - JIFFIES_TO_MS(tm->lastuse);
> > > +}
> > > +
> > 
> > And this, much better.
> > > +bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs);
> > > +put_32aligned_u64(>n_packets, bs->packets);
> > > +put_32aligned_u64(>n_bytes, bs->bytes);
> > 
> > 
> > > +int
> > > +tc_dump_flower_start(int ifindex, struct nl_dump *dump)
> > > +{
> > > +struct ofpbuf request;
> > > +struct tcmsg *tcmsg;
> > > +
> > > +tcmsg = tc_make_request(ifindex, RTM_GETTFILTER, NLM_F_DUMP, 
> > > );
> > > +tcmsg->tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS);
> > 
> > But that went in an opposite direction of the previous patch because
> > it is used like 5 times and it could be a define in tc.h leaving the TC
> > details hidden in there.
> > 
> 
> right. planned to do it but somehow skipped it. I'll be happy
> to update this if needed for this series or in a later commit.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V10 05/33] tc: Add tc flower functions

[Roi Dayan]

On 08/06/2017 14:46, Roi Dayan wrote:

Add tc helper functions to query and manipulate the flower classifier.

Signed-off-by: Paul Blakey 
Co-authored-by: Roi Dayan 
Signed-off-by: Roi Dayan 
---
 lib/tc.c | 989 +++
 lib/tc.h | 103 +++
 2 files changed, 1092 insertions(+)

diff --git a/lib/tc.c b/lib/tc.c
index 1f12e4a..30ece84 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, 
Inc.
+ * Copyright (c) 2016 Mellanox Technologies, Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,13 +18,26 @@
 #include 
 #include "tc.h"
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "byte-order.h"
 #include "netlink-socket.h"
 #include "netlink.h"
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vlog.h"
+#include "timeval.h"
+#include 

 VLOG_DEFINE_THIS_MODULE(tc);

+static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
+
 struct tcmsg *
 tc_make_request(int ifindex, int type, unsigned int flags,
 struct ofpbuf *request)
@@ -91,3 +105,978 @@ tc_add_del_ingress_qdisc(int ifindex, bool add)

 return 0;
 }
+
+static const struct nl_policy tca_policy[] = {
+[TCA_KIND] = { .type = NL_A_STRING, .optional = false, },
+[TCA_OPTIONS] = { .type = NL_A_NESTED, .optional = false, },
+[TCA_STATS] = { .type = NL_A_UNSPEC,
+.min_len = sizeof(struct tc_stats), .optional = true, },
+[TCA_STATS2] = { .type = NL_A_NESTED, .optional = true, },
+};
+
+static const struct nl_policy tca_flower_policy[] = {
+[TCA_FLOWER_CLASSID] = { .type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_INDEV] = { .type = NL_A_STRING, .max_len = IFNAMSIZ,
+   .optional = true, },
+[TCA_FLOWER_KEY_ETH_SRC] = { .type = NL_A_UNSPEC,
+ .min_len = ETH_ALEN, .optional = true, },
+[TCA_FLOWER_KEY_ETH_DST] = { .type = NL_A_UNSPEC,
+ .min_len = ETH_ALEN, .optional = true, },
+[TCA_FLOWER_KEY_ETH_SRC_MASK] = { .type = NL_A_UNSPEC,
+  .min_len = ETH_ALEN,
+  .optional = true, },
+[TCA_FLOWER_KEY_ETH_DST_MASK] = { .type = NL_A_UNSPEC,
+  .min_len = ETH_ALEN,
+  .optional = true, },
+[TCA_FLOWER_KEY_ETH_TYPE] = { .type = NL_A_U16, .optional = false, },
+[TCA_FLOWER_FLAGS] = { .type = NL_A_U32, .optional = false, },
+[TCA_FLOWER_ACT] = { .type = NL_A_NESTED, .optional = false, },
+[TCA_FLOWER_KEY_IP_PROTO] = { .type = NL_A_U8, .optional = true, },
+[TCA_FLOWER_KEY_IPV4_SRC] = { .type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_KEY_IPV4_DST] = {.type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_KEY_IPV4_SRC_MASK] = { .type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_KEY_IPV4_DST_MASK] = { .type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_KEY_IPV6_SRC] = { .type = NL_A_UNSPEC,
+  .min_len = sizeof(struct in6_addr),
+  .optional = true, },
+[TCA_FLOWER_KEY_IPV6_DST] = { .type = NL_A_UNSPEC,
+  .min_len = sizeof(struct in6_addr),
+  .optional = true, },
+[TCA_FLOWER_KEY_IPV6_SRC_MASK] = { .type = NL_A_UNSPEC,
+   .min_len = sizeof(struct in6_addr),
+   .optional = true, },
+[TCA_FLOWER_KEY_IPV6_DST_MASK] = { .type = NL_A_UNSPEC,
+   .min_len = sizeof(struct in6_addr),
+   .optional = true, },
+[TCA_FLOWER_KEY_TCP_SRC] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_TCP_DST] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_TCP_SRC_MASK] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_TCP_DST_MASK] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_UDP_SRC] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_UDP_DST] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_UDP_SRC_MASK] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_UDP_DST_MASK] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_VLAN_ID] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_VLAN_PRIO] = { .type = NL_A_U8, .optional = true, },
+[TCA_FLOWER_KEY_VLAN_ETH_TYPE] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_ENC_KEY_ID] = { .type = NL_A_U32, .optional = true, },
+[TCA_FLOWER_KEY_ENC_IPV4_SRC] = { .type = NL_A_U32, .optional = true, },
+

Re: [ovs-dev] [PATCH V10 05/33] tc: Add tc flower functions

[Roi Dayan]

On 09/06/2017 21:37, Flavio Leitner wrote:

On Thu, Jun 08, 2017 at 02:46:22PM +0300, Roi Dayan wrote:

Add tc helper functions to query and manipulate the flower classifier.

Signed-off-by: Paul Blakey 
Co-authored-by: Roi Dayan 
Signed-off-by: Roi Dayan 


Acked-by: Flavio Leitner 

Not sure why SCTP wasn't implemented, but not a blocker either.



Hi Flavio,
I didn't add it because later needed to spread changes across more
commits to support it to the end. planned to do it in a later
commit after the series.
All the other changes took me long enough and I wanted to minimize the
wait.




--- a/lib/tc.c
+++ b/lib/tc.c

[...]

+
+#define JIFFIES_TO_MS(x) (x * 10)
+};


Thanks for fixing this


+static void
+nl_parse_tcf(const struct tcf_t *tm, struct tc_flower *flower)
+{
+flower->lastused = time_msec() - JIFFIES_TO_MS(tm->lastuse);
+}
+


And this, much better.

+bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs);
+put_32aligned_u64(>n_packets, bs->packets);
+put_32aligned_u64(>n_bytes, bs->bytes);




+int
+tc_dump_flower_start(int ifindex, struct nl_dump *dump)
+{
+struct ofpbuf request;
+struct tcmsg *tcmsg;
+
+tcmsg = tc_make_request(ifindex, RTM_GETTFILTER, NLM_F_DUMP, );
+tcmsg->tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS);


But that went in an opposite direction of the previous patch because
it is used like 5 times and it could be a define in tc.h leaving the TC
details hidden in there.



right. planned to do it but somehow skipped it. I'll be happy
to update this if needed for this series or in a later commit.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V10 05/33] tc: Add tc flower functions

[Flavio Leitner]

On Thu, Jun 08, 2017 at 02:46:22PM +0300, Roi Dayan wrote:
> Add tc helper functions to query and manipulate the flower classifier.
> 
> Signed-off-by: Paul Blakey 
> Co-authored-by: Roi Dayan 
> Signed-off-by: Roi Dayan 

Acked-by: Flavio Leitner 

Not sure why SCTP wasn't implemented, but not a blocker either.

> --- a/lib/tc.c
> +++ b/lib/tc.c
[...]
> +
> +#define JIFFIES_TO_MS(x) (x * 10)
> +};

Thanks for fixing this

> +static void
> +nl_parse_tcf(const struct tcf_t *tm, struct tc_flower *flower)
> +{
> +flower->lastused = time_msec() - JIFFIES_TO_MS(tm->lastuse);
> +}
> +

And this, much better.
> +bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs);
> +put_32aligned_u64(>n_packets, bs->packets);
> +put_32aligned_u64(>n_bytes, bs->bytes);


> +int
> +tc_dump_flower_start(int ifindex, struct nl_dump *dump)
> +{
> +struct ofpbuf request;
> +struct tcmsg *tcmsg;
> +
> +tcmsg = tc_make_request(ifindex, RTM_GETTFILTER, NLM_F_DUMP, );
> +tcmsg->tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS);

But that went in an opposite direction of the previous patch because
it is used like 5 times and it could be a define in tc.h leaving the TC
details hidden in there.

-- 
Flavio

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev