[ovs-dev] [PATCH 5/9] Windows: Add conntrack-flush support in userspace

[Paul Boca]

Hi Sai!

A small nit inline.
Otherwise it looks fine to me.

Acked-by: Paul-Daniel Boca <pboca at cloudbasesolutions.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Sairam
> Venugopal
> Sent: Tuesday, June 21, 2016 4:23 AM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH 5/9] Windows: Add conntrack-flush support in
> userspace
> 
> Modify dpif-netlink.c and netlink-conntrack.c to send down flush command
> to Windows datapath. Include netlink-conntrack.c and netlink-conntrack.h
> in automake.mk for Windows binaries.
> 
> Windows currently supports only NETLINK_GENERIC port. In order to support
> the NETLINK_NETFILTER messages, the port id is being overwritten to
> NETLINK_GENERIC on Windows and datapath has been updated to support
> the
> new message format.
> 
> Signed-off-by: Sairam Venugopal <vsairam at vmware.com>
> ---
>  lib/automake.mk         |  2 ++
>  lib/dpif-netlink.c      | 15 +++------------
>  lib/netlink-conntrack.c | 33 +++++++++++++++++++++++++++++----
>  3 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index eabc0e7..4d4ee01 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -372,6 +372,8 @@ lib_libopenvswitch_la_SOURCES += \
>  	lib/dpif-netlink.c \
>  	lib/dpif-netlink.h \
>  	lib/netdev-windows.c \
> +	lib/netlink-conntrack.c \
> +	lib/netlink-conntrack.h \
>  	lib/netlink-notifier.c \
>  	lib/netlink-notifier.h \
>  	lib/netlink-protocol.h \
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 9bff3a8..e2bea23 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2274,7 +2274,6 @@ dpif_netlink_get_datapath_version(void)
>      return version_str;
>  }
> 
> -#ifdef __linux__
>  struct dpif_netlink_ct_dump_state {
>      struct ct_dpif_dump_state up;
>      struct nl_ct_dump_state *nl_ct_dump;
> @@ -2335,7 +2334,6 @@ dpif_netlink_ct_flush(struct dpif *dpif
> OVS_UNUSED, const uint16_t *zone)
>          return nl_ct_flush();
>      }
>  }
> -#endif
> 
>  const struct dpif_class dpif_netlink_class = {
>      "system",
> @@ -2377,17 +2375,10 @@ const struct dpif_class dpif_netlink_class = {
>      NULL,                       /* enable_upcall */
>      NULL,                       /* disable_upcall */
>      dpif_netlink_get_datapath_version, /* get_datapath_version */
> -#ifdef __linux__
>      dpif_netlink_ct_dump_start,
>      dpif_netlink_ct_dump_next,
>      dpif_netlink_ct_dump_done,
> -    dpif_netlink_ct_flush,
> -#else
> -    NULL,                       /* ct_dump_start */
> -    NULL,                       /* ct_dump_next */
> -    NULL,                       /* ct_dump_done */
> -    NULL,                       /* ct_flush */
> -#endif
> +    dpif_netlink_ct_flush
>  };
> 
>  static int
> @@ -2442,7 +2433,7 @@ dpif_netlink_is_internal_device(const char *name)
> 
>      return reply.type == OVS_VPORT_TYPE_INTERNAL;
>  }
> -

> +
>  /* Parses the contents of 'buf', which contains a "struct ovs_header"
> followed
>   * by Netlink attributes, into 'vport'.  Returns 0 if successful, otherwise a
>   * positive errno value.
> @@ -2946,7 +2937,7 @@ dpif_netlink_flow_get_stats(const struct
> dpif_netlink_flow *flow,
>      stats->used = flow->used ? get_32aligned_u64(flow->used) : 0;
>      stats->tcp_flags = flow->tcp_flags ? *flow->tcp_flags : 0;
>  }
> -

> +
>  /* Logs information about a packet that was recently lost in 'ch' (in
>   * 'dpif_'). */
>  static void
> diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
> index 47a3c62..bc31391 100644
> --- a/lib/netlink-conntrack.c
> +++ b/lib/netlink-conntrack.c
> @@ -75,6 +75,10 @@ static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(1, 5);
>  #define IPS_UNTRACKED_BIT 12
>  #define IPS_UNTRACKED (1 << IPS_UNTRACKED_BIT)
> 
> +#ifdef _WIN32
> +#define NETLINK_NETFILTER       NETLINK_GENERIC //Override for Windowss
[Paul Boca] Overriding without #undef would trigger a warning at compile time.
*And one 's' in Windows.

> +#endif
> +
>  static const struct nl_policy nfnlgrp_conntrack_policy[] = {
>      [CTA_TUPLE_ORIG] = { .type = NL_A_NESTED, .optional = false },
>      [CTA_TUPLE_REPLY] = { .type = NL_A_NESTED, .optional = false },
> @@ -118,7 +122,7 @@ struct nl_ct_dump_state {
>      bool filter_zone;
>      uint16_t zone;
>  };
> -

> +
>  /* Conntrack netlink dumping. */
> 
>  /* Initialize a conntrack netlink dump. */
> @@ -200,7 +204,7 @@ nl_ct_dump_done(struct nl_ct_dump_state *state)
>      free(state);
>      return error;
>  }
> -

> +
>  /* Format conntrack event 'entry' of 'type' to 'ds'. */
>  void
>  nl_ct_format_event_entry(const struct ct_dpif_entry *entry,
> @@ -235,6 +239,26 @@ nl_ct_flush(void)
>      return err;
>  }
> 
> +#ifdef _WIN32
> +int
> +nl_ct_flush_zone(uint16_t flush_zone)
> +{
> +    /* Windows can flush a specific zone */
> +    struct ofpbuf buf;
> +    int err;
> +
> +    ofpbuf_init(&buf, NL_DUMP_BUFSIZE);
> +
> +    nl_msg_put_nfgenmsg(&buf, 0, AF_UNSPEC, NFNL_SUBSYS_CTNETLINK,
> +                        IPCTNL_MSG_CT_DELETE, NLM_F_REQUEST);
> +    nl_msg_put_be16(&buf, CTA_ZONE, flush_zone);
> +
> +    err = nl_transact(NETLINK_NETFILTER, &buf, NULL);
> +    ofpbuf_uninit(&buf);
> +
> +    return err;
> +}
> +#else
>  int
>  nl_ct_flush_zone(uint16_t flush_zone)
>  {
> @@ -299,7 +323,8 @@ nl_ct_flush_zone(uint16_t flush_zone)
>       * have a master connection anymore */
>      return 0;
>  }
> -

> +#endif
> +
>  /* Conntrack netlink parsing. */
> 
>  static bool
> @@ -788,7 +813,7 @@ nl_ct_parse_entry(struct ofpbuf *buf, struct
> ct_dpif_entry *entry,
> 
>      return true;
>  }
> -

> +
>  /* NetFilter utility functions. */
> 
>  /* Puts a nlmsghdr and nfgenmsg at the beginning of 'msg', which must be
> --
> 2.5.0.windows.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev