Thanks Justin By the way, I just noticed that you reviewed most patches on Aug 1. Since the company e-mail is filtering some OVS list e-mails and I have relied on it just for notifications and was otherwise busy, I missed those. I am going through those carefully now.
One outstanding comment inline that I wanted to double check. On Fri, Aug 10, 2018 at 8:00 PM, Justin Pettit <[email protected]> wrote: > > > On Jul 16, 2018, at 4:39 PM, Darrell Ball <[email protected]> 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. > I re-ran Travis Bssically the same code as V8 Here is the link: Travis: https://travis-ci.org/darball/ovs_master The corresponding public git repo branch: https://github.com/darball/ovs_master/commits/conntrack Possibility, the build issue you saw is related to your proposed change in removing #include "ipf.h" from patch 7. diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h index 6eb55b4b0197..f8a31921e16a 100644 --- a/lib/ct-dpif.h +++ b/lib/ct-dpif.h @@ -17,7 +17,6 @@ #ifndef CT_DPIF_H #define CT_DPIF_H -#include "ipf.h" #include "openvswitch/types.h" #include "packets.h" ipf.h has the declaration for ipf_dump_ctx. > > > 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=<clear > ed>,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=<clear > ed>,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=<clear > ed>,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=<clear > ed>,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=<clear > ed>,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=<clear > ed>,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=<clear > ed>,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_compl > eted_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_compl > eted_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); > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
