[ovs-dev] [patch v8 9/9] ipf: Add fragmentation status reporting.

Justin Pettit jpettit at ovn.org
Sat Aug 11 03:00:29 UTC 2018


> On Jul 16, 2018, at 4:39 PM, Darrell Ball <dlu998 at gmail.com> wrote:
> 
> void
> ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
> {
> diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
> index f886ab9..2ff7e26 100644
> --- a/lib/ct-dpif.h
> +++ b/lib/ct-dpif.h
> @@ -204,6 +204,15 @@ int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns);
> int ct_dpif_ipf_set_enabled(struct dpif *, bool v6, bool enable);
> int ct_dpif_ipf_set_min_frag(struct dpif *, bool, uint32_t);
> int ct_dpif_ipf_set_max_nfrags(struct dpif *, uint32_t);
> +int ct_dpif_ipf_get_status(struct dpif *dpif, bool *, unsigned int *,
> +                           unsigned int *, unsigned int *, unsigned int *,
> +                           unsigned int *, unsigned int *, unsigned int *,
> +                           unsigned int *, bool *, unsigned int *,
> +                           unsigned int *, unsigned int *, unsigned int *,
> +                           unsigned int *, unsigned int *);
> +int ct_dpif_ipf_dump_start(struct dpif *dpif, struct ipf_dump_ctx **);

On my system, I needed to declare "ipf_dump_ctx" for the build to finish.

> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 5ec36bd..b3e7ce7 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> ..
> +static int
> +dpctl_ct_ipf_get_status(int argc, const char *argv[],
> +                        struct dpctl_params *dpctl_p)
> +{
> +    struct dpif *dpif;
> +    int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
> +    if (!error) {
> +        bool ipf_v4_enabled;
> +        unsigned int min_v4_frag_size;
> +        unsigned int nfrag_max;
> +        unsigned int nfrag;
> +        unsigned int n4frag_accepted;
> +        unsigned int n4frag_completed_sent;
> +        unsigned int n4frag_expired_sent;
> +        unsigned int n4frag_too_small;
> +        unsigned int n4frag_overlap;
> +        unsigned int min_v6_frag_size;
> +        bool ipf_v6_enabled;
> +        unsigned int n6frag_accepted;
> +        unsigned int n6frag_completed_sent;
> +        unsigned int n6frag_expired_sent;
> +        unsigned int n6frag_too_small;
> +        unsigned int n6frag_overlap;
> +        error = ct_dpif_ipf_get_status(dpif, &ipf_v4_enabled,
> +            &min_v4_frag_size, &nfrag_max, &nfrag, &n4frag_accepted,
> +            &n4frag_completed_sent, &n4frag_expired_sent, &n4frag_too_small,
> +            &n4frag_overlap, &ipf_v6_enabled, &min_v6_frag_size,
> +            &n6frag_accepted, &n6frag_completed_sent, &n6frag_expired_sent,
> +            &n6frag_too_small, &n6frag_overlap);
> +

If we just use 'ipf_status', we can delete a lot of these variable declarations.

> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 76bc1d9..db551ea 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -6553,6 +6553,60 @@ dpif_netdev_ipf_set_max_nfrags(struct dpif *dpif OVS_UNUSED,
>     return ipf_set_max_nfrags(max_frags);
> }
> 
> +static int
> +dpif_netdev_ipf_get_status(struct dpif *dpif OVS_UNUSED,
> +    bool *ipf_v4_enabled, unsigned int *min_v4_frag_size,
> +    unsigned int *nfrag_max, unsigned int *nfrag,
> +    unsigned int *n4frag_accepted, unsigned int *n4frag_completed_sent,
> +    unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
> +    unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
> +    unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
> +    unsigned int *n6frag_completed_sent, unsigned int *n6frag_expired_sent,
> +    unsigned int *n6frag_too_small, unsigned int *n6frag_overlap)
> +{
> +    struct ipf_status ipf_status;
> +    ipf_get_status(&ipf_status);
> +    *ipf_v4_enabled = ipf_status.ifp_v4_enabled;
> +    *min_v4_frag_size = ipf_status.min_v4_frag_size;
> +    *nfrag_max = ipf_status.nfrag_max;
> +    *nfrag = ipf_status.nfrag;
> +    *n4frag_accepted = ipf_status.n4frag_accepted;
> +    *n4frag_completed_sent = ipf_status.n4frag_completed_sent;
> +    *n4frag_expired_sent = ipf_status.n4frag_expired_sent;
> +    *n4frag_too_small = ipf_status.n4frag_too_small;
> +    *n4frag_overlap = ipf_status.n4frag_overlap;
> +    *ipf_v6_enabled = ipf_status.ifp_v6_enabled;
> +    *min_v6_frag_size = ipf_status.min_v6_frag_size;
> +    *n6frag_accepted = ipf_status.n6frag_accepted;
> +    *n6frag_completed_sent = ipf_status.n6frag_completed_sent;
> +    *n6frag_expired_sent = ipf_status.n6frag_expired_sent;
> +    *n6frag_too_small = ipf_status.n6frag_too_small;
> +    *n6frag_overlap = ipf_status.n6frag_overlap;
> +    return 0;
> +}

As, I'll suggest later, this can be reduced to just a couple of lines if 'ipf_status' is passed into this function.

> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index 214f570..2e0d596 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -451,6 +451,22 @@ struct dpif_class {
>     int (*ipf_set_min_frag)(struct dpif *, bool v6, uint32_t min_frag);
>     /* Set maximum number of fragments tracked. */
>     int (*ipf_set_max_nfrags)(struct dpif *, uint32_t max_nfrags);
> +    /* Get fragmentation configuration status and counters. */
> +    int (*ipf_get_status)(struct dpif *, bool *ipf_v4_enabled,
> +        unsigned int *min_v4_frag_size,
> +        unsigned int *nfrag_max, unsigned int *nfrag,
> +        unsigned int *n4frag_accepted, unsigned int *n4frag_completed_sent,
> +        unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
> +        unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
> +        unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
> +        unsigned int *n6frag_completed_sent,
> +        unsigned int *n6frag_expired_sent, unsigned int *n6frag_too_small,
> +        unsigned int *n6frag_overlap);

I'd suggest passing in 'ipf_status', since it makes this prototype much smaller and has all the information you need.

> +    int (*ipf_dump_start)(struct dpif *, struct ipf_dump_ctx **ipf_dump_ctx);
> +    /* Finds the next ipf list and creates a string representation of the
> +     * state of an ipf list, to which 'dump' is pointed to. */

This comment should probably go above the declaration of ipf_dump_start().

> +    int (*ipf_dump_next)(struct dpif *, void *, char **dump);
> +    int (*ipf_dump_done)(struct dpif *, void *ipf_dump_ctx);

> diff --git a/lib/ipf.h b/lib/ipf.h
> index 4289e5e..36a182a 100644
> --- a/lib/ipf.h
> +++ b/lib/ipf.h
> @@ -63,4 +63,14 @@ int ipf_set_min_frag(bool v6, uint32_t value);
> 
> int ipf_set_max_nfrags(uint32_t value);
> 

We need to reintroduce 'ipf_status', since this is where it's first used:

> +struct ipf_status {
> +   bool ifp_v4_enabled;
> +   unsigned int min_v4_frag_size;
> +   unsigned int nfrag_max;
> +   unsigned int nfrag;
> +   unsigned int n4frag_accepted;
> +   unsigned int n4frag_completed_sent;
> +   unsigned int n4frag_expired_sent;
> +   unsigned int n4frag_too_small;
> +   unsigned int n4frag_overlap;
> +   bool ifp_v6_enabled;
> +   unsigned int min_v6_frag_size;
> +   unsigned int n6frag_accepted;
> +   unsigned int n6frag_completed_sent;
> +   unsigned int n6frag_expired_sent;
> +   unsigned int n6frag_too_small;
> +   unsigned int n6frag_overlap;
> +};

I think it's a little cleaner if we create another struct that consolidates the common elements between IPv4 and IPv6.  Something like the following:

-=-=-=-=-=-=-=-
struct proto_status {
    bool enabled;
    unsigned int min_frag_size;

    /* Fragment statistics */
    unsigned int accepted;
    unsigned int completed_sent;
    unsigned int expired_sent;
    unsigned int too_small;
    unsigned int overlap;
};

struct ipf_status {
    unsigned int nfrag;
    unsigned int nfrag_max;

    struct proto_status v4;
    struct proto_status v6;
};
-=-=-=-=-=-=-=-

I worry a bit about wrapping 32-bit counters.  I see that the current atomic_count is an unsigned int, but It might be worth creating a new 64-bit type so that we can handle things like "accepted", which may be fairly high.

> diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
> index 40b7567..fcae3cc 100644
> --- a/tests/system-userspace-macros.at
> +++ b/tests/system-userspace-macros.at
> ...
> +# FORMAT_FRAG_LIST([])
> +#
> +# Strip content from the piped input which can differ from test to test; recirc_id
> +# and ip_id fields in an ipf_list vary from test to test and hence are cleared.
> +m4_define([FORMAT_FRAG_LIST],
> +    [[sed -e 's/ip_id=[0-9]*/ip_id=<cleared>/g' -e 's/recirc_id=[0-9]*/recirc_id=<cleared>/g']])
> +
> +# DPCTL_CHECK_FRAGMENTATION_FAIL()
> +#
> +# Used to check fragmentation counters for some fragmentation tests using
> +# the userspace datapath, when failure to transmit fragments is expected.
> +m4_define([DPCTL_CHECK_FRAGMENTATION_FAIL],
> +[
> +AT_CHECK([ovs-appctl dpctl/ipf-get-status -m | FORMAT_FRAG_LIST()], [], [dnl
> +        Fragmentation Module Status
> +        ---------------------------
> +        v4 enabled: 1
> +        v6 enabled: 1
> +        max num frags (v4/v6): 500
> +        num frag: 7
> +        min v4 frag size: 1000
> +        v4 frags accepted: 7
> +        v4 frags completed: 0
> +        v4 frags expired: 0
> +        v4 frags too small: 0
> +        v4 frags overlapped: 0
> +        min v6 frag size: 1280
> +        v6 frags accepted: 0
> +        v6 frags completed: 0
> +        v6 frags expired: 0
> +        v6 frags too small: 0
> +        v6 frags overlapped: 0
> +
> +        Fragment Lists:
> +
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)
> +(src=10.1.1.1,dst=10.1.1.2,recirc_id=<cleared>,ip_id=<cleared>,dl_type=0x800,zone=9,nw_proto=1,num_fragments=1,state=first frag)

If it's not too difficult, it might be nice to add some IPv6 stats here, too.

I've appended an incremental with some of the suggestions I made above as well as a few minor documentation/comment changes.

--Justin


-=-=-=-=-=-=-=-=-

diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index a59bc1e930d4..f5387c6a0678 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 
 #include "ct-dpif.h"
+#include "ipf.h"
 #include "openvswitch/ofp-parse.h"
 #include "openvswitch/vlog.h"
 
@@ -188,24 +189,10 @@ ct_dpif_ipf_set_max_nfrags(struct dpif *dpif, uint32_t max
_frags)
             : EOPNOTSUPP);
 }
 
-int ct_dpif_ipf_get_status(struct dpif *dpif, bool *ipf_v4_enabled,
-    unsigned int *min_v4_frag_size, unsigned int *nfrag_max,
-    unsigned int *nfrag, unsigned int *n4frag_accepted,
-    unsigned int *n4frag_completed_sent,
-    unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
-    unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
-    unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
-    unsigned int *n6frag_completed_sent,
-    unsigned int *n6frag_expired_sent, unsigned int *n6frag_too_small,
-    unsigned int *n6frag_overlap)
+int ct_dpif_ipf_get_status(struct dpif *dpif, struct ipf_status *status)
 {
     return (dpif->dpif_class->ipf_get_status
-            ? dpif->dpif_class->ipf_get_status(dpif, ipf_v4_enabled,
-            min_v4_frag_size, nfrag_max, nfrag, n4frag_accepted,
-            n4frag_completed_sent, n4frag_expired_sent, n4frag_too_small,
-            n4frag_overlap, ipf_v6_enabled, min_v6_frag_size, n6frag_accepted,
-            n6frag_completed_sent, n6frag_expired_sent, n6frag_too_small,
-            n6frag_overlap)
+            ? dpif->dpif_class->ipf_get_status(dpif, status)
             : EOPNOTSUPP);
 }
 
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 57d375bc1e82..603dbcf2d14a 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015 Nicira, Inc.
+ * Copyright (c) 2015, 2018 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,9 @@
 #include "openvswitch/types.h"
 #include "packets.h"
 
+struct ipf_dump_ctx;
+struct ipf_status;
+
 union ct_dpif_inet_addr {
     ovs_be32 ip;
     ovs_be32 ip6[4];
@@ -203,12 +206,7 @@ int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns)
;
 int ct_dpif_ipf_set_enabled(struct dpif *, bool v6, bool enable);
 int ct_dpif_ipf_set_min_frag(struct dpif *, bool, uint32_t);
 int ct_dpif_ipf_set_max_nfrags(struct dpif *, uint32_t);
-int ct_dpif_ipf_get_status(struct dpif *dpif, bool *, unsigned int *,
-                           unsigned int *, unsigned int *, unsigned int *,
-                           unsigned int *, unsigned int *, unsigned int *,
-                           unsigned int *, bool *, unsigned int *,
-                           unsigned int *, unsigned int *, unsigned int *,
-                           unsigned int *, unsigned int *);
+int ct_dpif_ipf_get_status(struct dpif *dpif, struct ipf_status *);
 int ct_dpif_ipf_dump_start(struct dpif *dpif, struct ipf_dump_ctx **);
 int ct_dpif_ipf_dump_next(struct dpif *dpif, void *, char **);
 int ct_dpif_ipf_dump_done(struct dpif *dpif, void *);
diff --git a/lib/dpctl.c b/lib/dpctl.c
index f650b37dd901..832220e5102c 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1799,61 +1799,41 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
     struct dpif *dpif;
     int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
     if (!error) {
-        bool ipf_v4_enabled;
-        unsigned int min_v4_frag_size;
-        unsigned int nfrag_max;
-        unsigned int nfrag;
-        unsigned int n4frag_accepted;
-        unsigned int n4frag_completed_sent;
-        unsigned int n4frag_expired_sent;
-        unsigned int n4frag_too_small;
-        unsigned int n4frag_overlap;
-        unsigned int min_v6_frag_size;
-        bool ipf_v6_enabled;
-        unsigned int n6frag_accepted;
-        unsigned int n6frag_completed_sent;
-        unsigned int n6frag_expired_sent;
-        unsigned int n6frag_too_small;
-        unsigned int n6frag_overlap;
-        error = ct_dpif_ipf_get_status(dpif, &ipf_v4_enabled,
-            &min_v4_frag_size, &nfrag_max, &nfrag, &n4frag_accepted,
-            &n4frag_completed_sent, &n4frag_expired_sent, &n4frag_too_small,
-            &n4frag_overlap, &ipf_v6_enabled, &min_v6_frag_size,
-            &n6frag_accepted, &n6frag_completed_sent, &n6frag_expired_sent,
-            &n6frag_too_small, &n6frag_overlap);
+        struct ipf_status status;
+        error = ct_dpif_ipf_get_status(dpif, &status);
 
         if (!error) {
             dpctl_print(dpctl_p, "        Fragmentation Module Status\n");
             dpctl_print(dpctl_p, "        ---------------------------\n");
-            dpctl_print(dpctl_p, "        v4 enabled: %u\n", ipf_v4_enabled);
-            dpctl_print(dpctl_p, "        v6 enabled: %u\n", ipf_v6_enabled);
+            dpctl_print(dpctl_p, "        v4 enabled: %u\n", status.v4.enabled);
+            dpctl_print(dpctl_p, "        v6 enabled: %u\n", status.v6.enabled);
             dpctl_print(dpctl_p, "        max num frags (v4/v6): %u\n",
-                        nfrag_max);
-            dpctl_print(dpctl_p, "        num frag: %u\n", nfrag);
+                        status.nfrag_max);
+            dpctl_print(dpctl_p, "        num frag: %u\n", status.nfrag);
             dpctl_print(dpctl_p, "        min v4 frag size: %u\n",
-                        min_v4_frag_size);
+                        status.v4.min_frag_size);
             dpctl_print(dpctl_p, "        v4 frags accepted: %u\n",
-                        n4frag_accepted);
+                        status.v4.accepted);
             dpctl_print(dpctl_p, "        v4 frags completed: %u\n",
-                        n4frag_completed_sent);
+                        status.v4.completed_sent);
             dpctl_print(dpctl_p, "        v4 frags expired: %u\n",
-                        n4frag_expired_sent);
+                        status.v4.expired_sent);
             dpctl_print(dpctl_p, "        v4 frags too small: %u\n",
-                        n4frag_too_small);
+                        status.v4.too_small);
             dpctl_print(dpctl_p, "        v4 frags overlapped: %u\n",
-                        n4frag_overlap);
+                        status.v4.overlap);
             dpctl_print(dpctl_p, "        min v6 frag size: %u\n",
-                        min_v6_frag_size);
+                        status.v6.min_frag_size);
             dpctl_print(dpctl_p, "        v6 frags accepted: %u\n",
-                        n6frag_accepted);
+                        status.v6.accepted);
             dpctl_print(dpctl_p, "        v6 frags completed: %u\n",
-                        n6frag_completed_sent);
+                        status.v6.completed_sent);
             dpctl_print(dpctl_p, "        v6 frags expired: %u\n",
-                        n6frag_expired_sent);
+                        status.v6.expired_sent);
             dpctl_print(dpctl_p, "        v6 frags too small: %u\n",
-                        n6frag_too_small);
+                        status.v6.too_small);
             dpctl_print(dpctl_p, "        v6 frags overlapped: %u\n",
-                        n6frag_overlap);
+                        status.v6.overlap);
         } else {
             dpctl_error(dpctl_p, error,
                         "ipf status could not be retrieved");
diff --git a/lib/dpctl.man b/lib/dpctl.man
index fc53094d2dc4..db2b7f8bcc4b 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -304,5 +304,6 @@ fragmentation is enabled.  Only supported for userspace datapath.
 .TP
 .DO "[\fB\-m\fR | \fB\-\-more\fR]" "\*(DX\fBipf\-get\-status\fR [\fIdp\fR]"
 Gets the configuration settings and fragment counters associated with the
-fragmentation handling of the userspace datapath connection tracker.  With
-\fB\-m\fR or \fB\-\-more\fR, also dumps the ipf lists.
+fragmentation handling of the userspace datapath connection tracker.
+With \fB\-m\fR or \fB\-\-more\fR, also dumps the IP fragment lists.
+Only supported for userspace datapath.
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 73ab632b969e..d2cd0cbfe2ec 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6801,33 +6801,9 @@ dpif_netdev_ipf_set_max_nfrags(struct dpif *dpif OVS_UNUSED,
 
 static int
 dpif_netdev_ipf_get_status(struct dpif *dpif OVS_UNUSED,
-    bool *ipf_v4_enabled, unsigned int *min_v4_frag_size,
-    unsigned int *nfrag_max, unsigned int *nfrag,
-    unsigned int *n4frag_accepted, unsigned int *n4frag_completed_sent,
-    unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
-    unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
-    unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
-    unsigned int *n6frag_completed_sent, unsigned int *n6frag_expired_sent,
-    unsigned int *n6frag_too_small, unsigned int *n6frag_overlap)
-{
-    struct ipf_status ipf_status;
-    ipf_get_status(&ipf_status);
-    *ipf_v4_enabled = ipf_status.ifp_v4_enabled;
-    *min_v4_frag_size = ipf_status.min_v4_frag_size;
-    *nfrag_max = ipf_status.nfrag_max;
-    *nfrag = ipf_status.nfrag;
-    *n4frag_accepted = ipf_status.n4frag_accepted;
-    *n4frag_completed_sent = ipf_status.n4frag_completed_sent;
-    *n4frag_expired_sent = ipf_status.n4frag_expired_sent;
-    *n4frag_too_small = ipf_status.n4frag_too_small;
-    *n4frag_overlap = ipf_status.n4frag_overlap;
-    *ipf_v6_enabled = ipf_status.ifp_v6_enabled;
-    *min_v6_frag_size = ipf_status.min_v6_frag_size;
-    *n6frag_accepted = ipf_status.n6frag_accepted;
-    *n6frag_completed_sent = ipf_status.n6frag_completed_sent;
-    *n6frag_expired_sent = ipf_status.n6frag_expired_sent;
-    *n6frag_too_small = ipf_status.n6frag_too_small;
-    *n6frag_overlap = ipf_status.n6frag_overlap;
+                           struct ipf_status *status)
+{
+    ipf_get_status(status);
     return 0;
 }
 
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 2e0d5968cea6..457911b7277a 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -452,19 +452,11 @@ struct dpif_class {
     /* Set maximum number of fragments tracked. */
     int (*ipf_set_max_nfrags)(struct dpif *, uint32_t max_nfrags);
     /* Get fragmentation configuration status and counters. */
-    int (*ipf_get_status)(struct dpif *, bool *ipf_v4_enabled,
-        unsigned int *min_v4_frag_size,
-        unsigned int *nfrag_max, unsigned int *nfrag,
-        unsigned int *n4frag_accepted, unsigned int *n4frag_completed_sent,
-        unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
-        unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
-        unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
-        unsigned int *n6frag_completed_sent,
-        unsigned int *n6frag_expired_sent, unsigned int *n6frag_too_small,
-        unsigned int *n6frag_overlap);
-    int (*ipf_dump_start)(struct dpif *, struct ipf_dump_ctx **ipf_dump_ctx);
+    int (*ipf_get_status)(struct dpif *, struct ipf_status *);
+
     /* Finds the next ipf list and creates a string representation of the
      * state of an ipf list, to which 'dump' is pointed to. */
+    int (*ipf_dump_start)(struct dpif *, struct ipf_dump_ctx **ipf_dump_ctx);
     int (*ipf_dump_next)(struct dpif *, void *, char **dump);
     int (*ipf_dump_done)(struct dpif *, void *ipf_dump_ctx);
 
diff --git a/lib/ipf.c b/lib/ipf.c
index ff08f5933e96..ff4de524abf2 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -1347,26 +1347,22 @@ ipf_set_max_nfrags(uint32_t value)
 int
 ipf_get_status(struct ipf_status *ipf_status)
 {
-    atomic_read_relaxed(&ifp_v4_enabled, &ipf_status->ifp_v4_enabled);
-    atomic_read_relaxed(&min_v4_frag_size, &ipf_status->min_v4_frag_size);
+    atomic_read_relaxed(&ifp_v4_enabled, &ipf_status->v4.enabled);
+    atomic_read_relaxed(&min_v4_frag_size, &ipf_status->v4.min_frag_size);
     atomic_read_relaxed(&nfrag_max, &ipf_status->nfrag_max);
     ipf_status->nfrag = atomic_count_get(&nfrag);
-    ipf_status->n4frag_accepted = atomic_count_get(&n4frag_accepted);
-    ipf_status->n4frag_completed_sent =
-        atomic_count_get(&n4frag_completed_sent);
-    ipf_status->n4frag_expired_sent =
-        atomic_count_get(&n4frag_expired_sent);
-    ipf_status->n4frag_too_small = atomic_count_get(&n4frag_too_small);
-    ipf_status->n4frag_overlap = atomic_count_get(&n4frag_overlap);
-    atomic_read_relaxed(&ifp_v6_enabled, &ipf_status->ifp_v6_enabled);
-    atomic_read_relaxed(&min_v6_frag_size, &ipf_status->min_v6_frag_size);
-    ipf_status->n6frag_accepted = atomic_count_get(&n6frag_accepted);
-    ipf_status->n6frag_completed_sent =
-        atomic_count_get(&n6frag_completed_sent);
-    ipf_status->n6frag_expired_sent =
-        atomic_count_get(&n6frag_expired_sent);
-    ipf_status->n6frag_too_small = atomic_count_get(&n6frag_too_small);
-    ipf_status->n6frag_overlap = atomic_count_get(&n6frag_overlap);
+    ipf_status->v4.accepted = atomic_count_get(&n4frag_accepted);
+    ipf_status->v4.completed_sent = atomic_count_get(&n4frag_completed_sent);
+    ipf_status->v4.expired_sent = atomic_count_get(&n4frag_expired_sent);
+    ipf_status->v4.too_small = atomic_count_get(&n4frag_too_small);
+    ipf_status->v4.overlap = atomic_count_get(&n4frag_overlap);
+    atomic_read_relaxed(&ifp_v6_enabled, &ipf_status->v6.enabled);
+    atomic_read_relaxed(&min_v6_frag_size, &ipf_status->v6.min_frag_size);
+    ipf_status->v6.accepted = atomic_count_get(&n6frag_accepted);
+    ipf_status->v6.completed_sent = atomic_count_get(&n6frag_completed_sent);
+    ipf_status->v6.expired_sent = atomic_count_get(&n6frag_expired_sent);
+    ipf_status->v6.too_small = atomic_count_get(&n6frag_too_small);
+    ipf_status->v6.overlap = atomic_count_get(&n6frag_overlap);
     return 0;
 }
 
@@ -1374,7 +1370,8 @@ struct ipf_dump_ctx {
     struct hmap_position bucket_pos;
 };
 
-/* Allocates an 'ipf_dump_ctx' to keep track of an hmap position. */
+/* Allocates an 'ipf_dump_ctx' to keep track of an hmap position. The
+ * caller must call ipf_dump_done() when dumping is finished. */
 int
 ipf_dump_start(struct ipf_dump_ctx **ipf_dump_ctx)
 {
@@ -1415,7 +1412,7 @@ ipf_dump_create(const struct ipf_list *ipf_list, struct ds *ds)
 
 /* Finds the next ipf list starting from 'ipf_dump_ctx->bucket_pos' and uses
  * ipf_dump_create() to create a string representation of the state of an
- * ipf list, to which 'dump' is pointed to. */
+ * ipf list, to which 'dump' is pointed to.  Return EOF when */
 int
 ipf_dump_next(struct ipf_dump_ctx *ipf_dump_ctx, char **dump)
 {
@@ -1439,7 +1436,7 @@ ipf_dump_next(struct ipf_dump_ctx *ipf_dump_ctx, char **dump)
     }
 }
 
-/* Frees an ipf_dump_ctx allocated by ipf_dump_start. */
+/* Frees 'ipf_dump_ctx' allocated by ipf_dump_start(). */
 int
 ipf_dump_done(struct ipf_dump_ctx *ipf_dump_ctx)
 {
diff --git a/lib/ipf.h b/lib/ipf.h
index 635988b2fcd8..8fa58ab75851 100644
--- a/lib/ipf.h
+++ b/lib/ipf.h
@@ -20,6 +20,26 @@
 #include "dp-packet.h"
 #include "openvswitch/types.h"
 
+struct proto_status {
+    bool enabled;
+    unsigned int min_frag_size;
+
+    /* Fragment statistics */
+    unsigned int accepted;
+    unsigned int completed_sent;
+    unsigned int expired_sent;
+    unsigned int too_small;
+    unsigned int overlap;
+};
+
+struct ipf_status {
+    unsigned int nfrag;
+    unsigned int nfrag_max;
+
+    struct proto_status v4;
+    struct proto_status v6;
+};
+
 void ipf_preprocess_conntrack(struct dp_packet_batch *pb, long long now,
                               ovs_be16 dl_type, uint16_t zone,
                               uint32_t hash_basis);





More information about the dev mailing list