diff mbox series

[ovs-dev] dpif-netlink: Add meter support.

Message ID 20180629035038.17920-1-jpettit@ovn.org
State Superseded
Headers show
Series [ovs-dev] dpif-netlink: Add meter support. | expand

Commit Message

Justin Pettit June 29, 2018, 3:50 a.m. UTC
From: Andy Zhou <azhou@ovn.org>

To work with kernel datapath that supports meter.

Signed-off-by: Andy Zhou <azhou@ovn.org>
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
This is Andy's patch.  I made a few fixes and cleanups, so I wouldn't
mind another set of eyes.

Also, this may have impact on the Windows port, since I don't believe it
has support for meters.  Unfortunately, I don't have a good way to test
Windows builds, so I would appreciate if someone from the Windows side
could look at this.
---
 lib/dpif-netlink.c | 300 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 280 insertions(+), 20 deletions(-)

Comments

0-day Robot June 29, 2018, 4:25 a.m. UTC | #1
Bleep bloop.  Greetings Justin Pettit, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Too many signoffs; are you missing Co-authored-by lines?
Lines checked: 356, Warnings: 0, Errors: 1


build:
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow    -Werror -MT lib/socket-util-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/socket-util-unix.lo lib/socket-util-unix.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/socket-util-unix.lo -MD -MP -MF lib/.deps/socket-util-unix.Tpo -c lib/socket-util-unix.c -o lib/socket-util-unix.o
depbase=`echo lib/stream-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow    -Werror -MT lib/stream-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/stream-unix.lo lib/stream-unix.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/stream-unix.lo -MD -MP -MF lib/.deps/stream-unix.Tpo -c lib/stream-unix.c -o lib/stream-unix.o
depbase=`echo lib/dpif-netlink.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow    -Werror -MT lib/dpif-netlink.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif-netlink.lo lib/dpif-netlink.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/dpif-netlink.lo -MD -MP -MF lib/.deps/dpif-netlink.Tpo -c lib/dpif-netlink.c -o lib/dpif-netlink.o
lib/dpif-netlink.c: In function 'dpif_netlink_meter_transact':
lib/dpif-netlink.c:2955:42: error: 'request' undeclared (first use in this function)
     error = nl_transact(NETLINK_GENERIC, request, replyp);
                                          ^
lib/dpif-netlink.c:2955:42: note: each undeclared identifier is reported only once for each function it appears in
lib/dpif-netlink.c:2946:44: error: unused parameter 'reuquest' [-Werror=unused-parameter]
 dpif_netlink_meter_transact(struct ofpbuf *reuquest, struct ofpbuf **replyp,
                                            ^
cc1: all warnings being treated as errors
make[2]: *** [lib/dpif-netlink.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index aa9bbd9463a7..a3bf0ac89492 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -212,6 +212,7 @@  static int ovs_datapath_family;
 static int ovs_vport_family;
 static int ovs_flow_family;
 static int ovs_packet_family;
+static int ovs_meter_family;
 
 /* Generic Netlink multicast groups for OVS.
  *
@@ -2921,40 +2922,296 @@  dpif_netlink_ct_flush(struct dpif *dpif OVS_UNUSED, const uint16_t *zone,
 
 /* Meters */
 static void
-dpif_netlink_meter_get_features(const struct dpif * dpif OVS_UNUSED,
+dpif_netlink_meter_init(struct dpif_netlink *dpif, struct ofpbuf *buf,
+                        void *stub, size_t size, uint32_t command)
+{
+    struct ovs_header *ovs_header;
+
+    ofpbuf_use_stub(buf, stub, size);
+
+    nl_msg_put_genlmsghdr(buf, 0, ovs_meter_family, NLM_F_REQUEST | NLM_F_ECHO,
+                          command, OVS_METER_VERSION);
+
+    ovs_header = ofpbuf_put_uninit(buf, sizeof *ovs_header);
+    ovs_header->dp_ifindex = dpif->dp_ifindex;
+}
+
+/* Execute meter 'request' in the kernel datapath.  If the command
+ * fails, returns a positive errno value.  Otherwise, stores the reply
+ * in '*replyp', parses the policy according to 'reply_policy' into the
+ * array of Netlink attribute in 'a', and returns 0.  On success, the
+ * caller is responsible for calling ofpbuf_delete() on '*replyp'
+ * ('replyp' will contain pointers into 'a'). */
+static int
+dpif_netlink_meter_transact(struct ofpbuf *reuquest, struct ofpbuf **replyp,
+                            const struct nl_policy *reply_policy,
+                            struct nlattr **a, size_t size_a)
+{
+    int error;
+    struct nlmsghdr *nlmsg;
+    struct genlmsghdr *genl;
+    struct ovs_header *ovs_header;
+
+    error = nl_transact(NETLINK_GENERIC, request, replyp);
+    ofpbuf_uninit(request);
+
+    if (error) {
+        VLOG_INFO("nl_transact for meter failed");
+        return error;
+    }
+
+    nlmsg = ofpbuf_try_pull(*replyp, sizeof *nlmsg);
+    genl = ofpbuf_try_pull(*replyp, sizeof *genl);
+    ovs_header = ofpbuf_try_pull(*replyp, sizeof *ovs_header);
+    if (!nlmsg || !genl || !ovs_header
+        || nlmsg->nlmsg_type != ovs_meter_family
+        || !nl_policy_parse(*replyp, 0, reply_policy, a, size_a)) {
+        VLOG_INFO("Kernel module response to meter tranaction is invalid");
+        return EINVAL;
+    }
+    return 0;
+}
+
+static void
+dpif_netlink_meter_get_features(const struct dpif *dpif_,
                                 struct ofputil_meter_features *features)
 {
-    features->max_meters = 0;
-    features->band_types = 0;
-    features->capabilities = 0;
-    features->max_bands = 0;
-    features->max_color = 0;
+    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+    struct ofpbuf buf, *msg;
+    uint64_t stub[1024 / 8];
+
+    static const struct nl_policy ovs_meter_features_policy[] = {
+        [OVS_METER_ATTR_MAX_METERS] = { .type = NL_A_U32 },
+        [OVS_METER_ATTR_MAX_BANDS] = { .type = NL_A_U32 },
+        [OVS_METER_ATTR_BANDS] = { .type = NL_A_NESTED, .optional = true },
+    };
+    struct nlattr *a[ARRAY_SIZE(ovs_meter_features_policy)];
+
+    memset(features, 0, sizeof *features);
+
+    dpif_netlink_meter_init(dpif, &buf, stub, sizeof stub,
+                            OVS_METER_CMD_FEATURES);
+    if (dpif_netlink_meter_transact(&buf, &msg, ovs_meter_features_policy, a,
+                                    ARRAY_SIZE(ovs_meter_features_policy))) {
+        return;
+    }
+
+    features->max_meters = nl_attr_get_u32(a[OVS_METER_ATTR_MAX_METERS]);
+    features->max_bands = nl_attr_get_u32(a[OVS_METER_ATTR_MAX_BANDS]);
+
+    /* Bands is a nested attribute of zero or more nested
+     * band attributes.  */
+    if (a[OVS_METER_ATTR_BANDS]) {
+        const struct nlattr *nla;
+        size_t left;
+
+        NL_NESTED_FOR_EACH (nla, left, a[OVS_METER_ATTR_BANDS]) {
+            const struct nlattr *band_nla;
+            size_t band_left;
+
+            NL_NESTED_FOR_EACH (band_nla, band_left, nla) {
+                if (nl_attr_type(band_nla) == OVS_BAND_ATTR_TYPE) {
+                    if (nl_attr_get_size(band_nla) == sizeof(uint32_t)) {
+                        switch (nl_attr_get_u32(band_nla)) {
+                        case OVS_METER_BAND_TYPE_DROP:
+                            features->band_types |= 1 << OFPMBT13_DROP;
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+    }
+    features->capabilities = OFPMF13_KBPS | OFPMF13_PKTPS | OFPMF13_BURST
+        | OFPMF13_STATS;
+
+    ofpbuf_delete(msg);
+}
+
+static int
+dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id *meter_id,
+                       struct ofputil_meter_config *config)
+{
+    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+    struct ofpbuf buf, *msg;
+    uint64_t stub[1024 / 8];
+    int error;
+    size_t i;
+
+    size_t bands_offset, band_offset;
+
+    static const struct nl_policy ovs_meter_set_response_policy[] = {
+        [OVS_METER_ATTR_ID] = { .type = NL_A_U32 },
+    };
+    struct nlattr *a[ARRAY_SIZE(ovs_meter_set_response_policy)];
+
+    /* Validate arguments. */
+    if (!(config->flags & (OFPMF13_KBPS | OFPMF13_PKTPS))) {
+        return EBADF; /* Rate unit type not set. */
+    }
+    if ((config->flags & OFPMF13_KBPS) && (config->flags & OFPMF13_PKTPS)) {
+        return EBADF; /* Both rate units may not be set. */
+    }
+    if (config->n_bands == 0) {
+        return EINVAL; /* No bands. */
+    }
+
+    dpif_netlink_meter_init(dpif, &buf, stub, sizeof stub, OVS_METER_CMD_SET);
+
+    if (meter_id->uint32 != UINT32_MAX) {
+        nl_msg_put_u32(&buf, OVS_METER_ATTR_ID, meter_id->uint32);
+    }
+    if (config->flags & OFPMF13_KBPS) {
+        nl_msg_put_flag(&buf, OVS_METER_ATTR_KBPS);
+    }
+
+    bands_offset = nl_msg_start_nested(&buf, OVS_METER_ATTR_BANDS);
+    /* Bands */
+    for (i = 0; i < config->n_bands; ++i) {
+        struct ofputil_meter_band * band = &config->bands[i];
+        uint32_t band_type;
+
+        band_offset = nl_msg_start_nested(&buf, OVS_BAND_ATTR_UNSPEC);
+
+        switch (band->type) {
+        case OFPMBT13_DROP:
+            band_type = OVS_METER_BAND_TYPE_DROP;
+            break;
+        default:
+            band_type = OVS_METER_BAND_TYPE_UNSPEC;
+        }
+        nl_msg_put_u32(&buf, OVS_BAND_ATTR_TYPE, band_type);
+        nl_msg_put_u32(&buf, OVS_BAND_ATTR_RATE, band->rate);
+        nl_msg_put_u32(&buf, OVS_BAND_ATTR_BURST,
+                       config->flags & OFPMF13_BURST ?
+                       band->burst_size : band->rate);
+        nl_msg_end_nested(&buf, band_offset);
+    }
+    nl_msg_end_nested(&buf, bands_offset);
+
+    error = dpif_netlink_meter_transact(&buf, &msg,
+                                    ovs_meter_set_response_policy, a,
+                                    ARRAY_SIZE(ovs_meter_set_response_policy));
+    if (error) {
+        VLOG_INFO("dpif_netlink_meter_tranact OVS_METER_CMD_SET failed");
+        return error;
+    }
+
+    meter_id->uint32 = nl_attr_get_u32(a[OVS_METER_ATTR_ID]);
+    ofpbuf_delete(msg);
+    return 0;
 }
 
+/* Retrieve statistics and/or delete meter 'meter_id'.  Statistics are
+ * stored in 'stats', if it is not null.  If 'command' is
+ * OVS_METER_CMD_DEL, the meter is deleted and statistics are optionally
+ * retrieved.  If 'command' is OVS_METER_CMD_GET, then statistics are
+ * simply retrieved. */
 static int
-dpif_netlink_meter_set(struct dpif *dpif OVS_UNUSED,
-                       ofproto_meter_id *meter_id OVS_UNUSED,
-                       struct ofputil_meter_config *config OVS_UNUSED)
+dpif_netlink_meter_get_stats(const struct dpif *dpif_,
+                             ofproto_meter_id meter_id,
+                             struct ofputil_meter_stats *stats,
+                             uint16_t max_bands,
+                             enum ovs_meter_cmd command)
 {
-    return EFBIG; /* meter_id out of range */
+    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+    struct ofpbuf buf, *msg;
+    uint64_t stub[1024 / 8];
+    int error;
+    size_t n_bands;
+
+    static const struct nl_policy ovs_meter_stats_policy[] = {
+        [OVS_METER_ATTR_ID] = { .type = NL_A_U32, .optional = true},
+        [OVS_METER_ATTR_STATS] = { NL_POLICY_FOR(struct ovs_flow_stats),
+                                   .optional = true},
+        [OVS_METER_ATTR_BANDS] = { .type = NL_A_NESTED, .optional = true },
+    };
+    struct nlattr *a[ARRAY_SIZE(ovs_meter_stats_policy)];
+
+    dpif_netlink_meter_init(dpif, &buf, stub, sizeof stub, command);
+
+    nl_msg_put_u32(&buf, OVS_METER_ATTR_ID, meter_id.uint32);
+
+    error = dpif_netlink_meter_transact(&buf, &msg,
+                                        ovs_meter_stats_policy, a,
+                                        ARRAY_SIZE(ovs_meter_stats_policy));
+    if (error) {
+        VLOG_INFO("dpif_netlink_meter_transact %s failed",
+                  command == OVS_METER_CMD_GET ? "get" : "del");
+        return error;
+    }
+
+    if (stats
+        && a[OVS_METER_ATTR_ID]
+        && a[OVS_METER_ATTR_STATS]
+        && nl_attr_get_u32(a[OVS_METER_ATTR_ID]) == meter_id.uint32) {
+        /* return stats */
+        const struct ovs_flow_stats *stat;
+        const struct nlattr *nla;
+        size_t left;
+
+        stat = nl_attr_get(a[OVS_METER_ATTR_STATS]);
+        stats->packet_in_count = get_32aligned_u64(&stat->n_packets);
+        stats->byte_in_count = get_32aligned_u64(&stat->n_bytes);
+
+        if (a[OVS_METER_ATTR_BANDS]) {
+            n_bands = 0;
+            NL_NESTED_FOR_EACH (nla, left, a[OVS_METER_ATTR_BANDS]) {
+                const struct nlattr *band_nla;
+                size_t band_left;
+
+                stat = NULL;
+
+                NL_NESTED_FOR_EACH (band_nla, band_left, nla) {
+                    if (nl_attr_type(band_nla) == OVS_BAND_ATTR_STATS
+                        && nl_attr_get_size(band_nla)
+                        == sizeof(struct ovs_flow_stats)) {
+                        stat = nl_attr_get(band_nla);
+                    }
+                }
+                if (!stat) {
+                    error = EINVAL;
+                    goto cleanup;
+                }
+
+                if (n_bands < max_bands) {
+                    stats->bands[n_bands].packet_count
+                        = get_32aligned_u64(&stat->n_packets);
+                    stats->bands[n_bands].byte_count
+                        = get_32aligned_u64(&stat->n_bytes);
+                    ++n_bands;
+                }
+            }
+            stats->n_bands = n_bands;
+        }
+    } else {
+        /* For a non-exist meter, return 0 stats. */
+        if (stats) {
+            stats->packet_in_count = 0;
+            stats->byte_in_count = 0;
+            stats->n_bands = 0;
+        }
+    }
+
+cleanup:
+    ofpbuf_delete(msg);
+    return error;
 }
 
 static int
-dpif_netlink_meter_get(const struct dpif *dpif OVS_UNUSED,
-                       ofproto_meter_id meter_id OVS_UNUSED,
-                       struct ofputil_meter_stats *stats OVS_UNUSED,
-                       uint16_t n_bands OVS_UNUSED)
+dpif_netlink_meter_get(const struct dpif *dpif, ofproto_meter_id meter_id,
+                       struct ofputil_meter_stats *stats, uint16_t max_bands)
 {
-    return EFBIG; /* meter_id out of range */
+    return dpif_netlink_meter_get_stats(dpif, meter_id, stats, max_bands,
+                                        OVS_METER_CMD_GET);
 }
 
 static int
-dpif_netlink_meter_del(struct dpif *dpif OVS_UNUSED,
-                       ofproto_meter_id meter_id OVS_UNUSED,
-                       struct ofputil_meter_stats *stats OVS_UNUSED,
-                       uint16_t n_bands OVS_UNUSED)
+dpif_netlink_meter_del(struct dpif *dpif, ofproto_meter_id meter_id,
+                       struct ofputil_meter_stats *stats, uint16_t max_bands)
 {
-    return EFBIG; /* meter_id out of range */
+    return dpif_netlink_meter_get_stats(dpif, meter_id, stats, max_bands,
+                                        OVS_METER_CMD_DEL);
 }
 
 
@@ -3040,6 +3297,9 @@  dpif_netlink_init(void)
             error = nl_lookup_genl_mcgroup(OVS_VPORT_FAMILY, OVS_VPORT_MCGROUP,
                                            &ovs_vport_mcgroup);
         }
+        if (!error) {
+            error = nl_lookup_genl_family(OVS_METER_FAMILY, &ovs_meter_family);
+        }
 
         ovs_tunnels_out_of_tree = dpif_netlink_rtnl_probe_oot_tunnels();