[ovs-dev] [PATCH 1/8] netdev-vport: Factor-out tunnel Push-pop code into separate module.

[Jesse Gross]

On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> new file mode 100644
> index 0000000..755b6c1
> --- /dev/null
> +++ b/lib/netdev-vport-private.h
[...]
> +struct vport_class {
> +    const char *dpif_port;
> +    struct netdev_class netdev_class;
> +};

Is this used anywhere outside of netdev-vport.c?

> diff --git a/lib/tnl-push-pop.h b/lib/tnl-push-pop.h
> new file mode 100644
> index 0000000..807cda6
> --- /dev/null
> +++ b/lib/tnl-push-pop.h

We might want to consider breaking this down even further into
individual tunnel implementations. At the very least, it seems like
STT is big and complicated enough to merit its own file.

Barring that, it might be useful to come up with another name for the
file. I'm not sure that push-pop is all that helpful for most people
without context. We could then also make sure that the function names
are consistent - for example, netdev_vport_range() doesn't give much
information about what the function does, especially if you don't know
that it's related to tunneling, and I would normally expect to find it
in netdev-vport.c.

> +int
> +netdev_geneve_build_header(const struct netdev *netdev,
> +                           struct ovs_action_push_tnl *data,
> +                           const struct flow *tnl_flow);
> +void
> +push_udp_header(struct dp_packet *packet,
> +                const struct ovs_action_push_tnl *data);
> +int
> +netdev_geneve_pop_header(struct dp_packet *packet);

This is really minor but it would be nice to have all of the functions
from a given protocol grouped together.