[ovs-dev] [PATCH v2 01/11] ct-dpif: New module.
Joe Stringer
joestringer at nicira.com
Tue Nov 17 22:26:59 UTC 2015
On 5 November 2015 at 19:12, Daniele Di Proietto <diproiettod at 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 <jrajahalme at nicira.com>
> Signed-off-by: Daniele Di Proietto <diproiettod at 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[] = {
More information about the dev
mailing list