> On Aug 13, 2018, at 2:19 PM, Darrell Ball <[email protected]> wrote: > > > 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. > > > I understand; I commented earlier about this. The reason for not using > ipf_status is not because I like typing lots. > > The reasoning is: > > 1/ ipf_status is specific to the userspace datapath. The kernel datapath and > various hardware offload > datapaths may not use or even be able to use the same datatype. > Note that dpctl and dpif code is intended to be generic across datapaths. > > 2/ Using ipf_status exposes what is intended to be private internal > datastructures to external modules > I was trying to avoid that, if possible, and use base types in common code. > > Perhaps, the easiest way to address this to simply have a dpif_ipf_status, > which is defined similarly to ipf_status, at > least initially ? These can diverge later if needed,
Yes, I had those same thoughts, so I think just creating a separate "dpif_ipf_status" would be a good solutions. --Justin _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
