On Thu, Jan 21, 2016 at 12:56 PM, Jesse Gross <je...@kernel.org> wrote:
> On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshe...@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?
>

netdev_vport_cast() depends on its definition, so I had to move it to
the header.


>> 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.
>
ok.

> 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.
>
ok, How about netdev-native-tnl.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.
ok.

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to