On 5 November 2015 at 19:12, Daniele Di Proietto <diproiet...@vmware.com> wrote: > This defines some structures (and their related formatting functions) to > manipulate entries in connection tracking tables. > > It will be used by next commits. > > Based on original work by Jarno Rajahalme > > Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> > Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
One thought that came to mind, I don't know if this might help the parseability of the output but should we comma-separate all the ct info when formatting it? I think that would be more consistent with the other parts of OVS. Minor extra comments: > +static void > +ct_dpif_format_ipproto(struct ds *ds, uint16_t ipproto) > +{ > + const char *name; > + > + name = (ipproto == IPPROTO_ICMP) ? "icmp" > + : (ipproto == IPPROTO_ICMPV6) ? "icmpv6" > + : (ipproto == IPPROTO_TCP) ? "tcp" > + : (ipproto == IPPROTO_UDP) ? "udp" > + : (ipproto == IPPROTO_SCTP) ? "sctp" > + : NULL; Is it worth sharing this with the similar code in match_format()? It could live in lib/packets.h. I had some minor style changes when I went through this, mostly what I saw as redundant comments: diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c index 9a23a5cf65ec..d63e7a1de40b 100644 --- a/lib/ct-dpif.c +++ b/lib/ct-dpif.c @@ -118,7 +118,6 @@ ct_dpif_flush(struct dpif *dpif, const uint16_t *zone) : EOPNOTSUPP); } -/* Free memory held by 'entry'. */ void ct_dpif_entry_uninit(struct ct_dpif_entry *entry) { @@ -129,9 +128,6 @@ ct_dpif_entry_uninit(struct ct_dpif_entry *entry) } } ^L -/* Conntrack entry formatting. */ - -/* Format conntrack 'entry' of 'type' to 'ds'. */ void ct_dpif_format_entry(const struct ct_dpif_entry *entry, struct ds *ds, bool verbose, bool print_stats) @@ -186,8 +182,6 @@ ct_dpif_format_entry(const struct ct_dpif_entry *entry, struct ds *ds, ds_put_cstr(ds, ")"); } } -^L -/* Formatters for the parts of the conntrack entries. */ static void ct_dpif_format_ipproto(struct ds *ds, uint16_t ipproto) @@ -283,17 +277,15 @@ static void ct_dpif_format_flags(struct ds *ds, const char *title, uint32_t flags, const struct flags *table) { - bool first = true; - if (title) { ds_put_cstr(ds, title); } for (; table->name; table++) { if (flags & table->flag) { - ds_put_format(ds, first ? "%s" : ",%s", table->name); - first = false; + ds_put_format(ds, "%s,", table->name); } } + ds_chomp(ds, ','); } static const struct flags tcp_flags[] = { _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev