On Tue, Apr 7, 2015 at 4:22 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Mon, Mar 30, 2015 at 3:14 PM, Jesse Gross <je...@nicira.com> wrote:
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 0c9f5a4..ef96862 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -61,6 +61,11 @@ static struct vlog_rate_limit err_rl =
>> VLOG_RATE_LIMIT_INIT(60, 5);
>> sizeof(struct udp_header) + \
>> sizeof(struct vxlanhdr))
>>
> ...
>
>> +
>> + if (gnh->proto_type != htons(ETH_TYPE_TEB)) {
>> + VLOG_WARN_RL(&err_rl, "unknown geneve encapsulated protocol: %#x\n",
>> + ntohs(gnh->proto_type));
>> + reset_tnl_md(md);
>> + return;
>> + }
>> +
>> + tnl->flags |= gnh->oam ? FLOW_TNL_F_OAM : 0;
>> + tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gnh->vni)) >> 8);
>> +
>
> We need to set FLOW_TNL_F_KEY. It is also missing for vxlan.
Fixed for both, thanks.
>> + dp_packet_reset_packet(packet, hlen);
>> +}
>> +
>> +static int
>> +netdev_geneve_pop_header(struct netdev *netdev_ OVS_UNUSED,
> ....
>
>> +
>> +static int
>> +netdev_geneve_push_header(const struct netdev *netdev OVS_UNUSED,
>> + struct dp_packet **packets, int cnt,
>> + const struct ovs_action_push_tnl *data)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < cnt; i++) {
>> + push_udp_header(packets[i], data->header, data->header_len);
>> + packets[i]->md =
>> PKT_METADATA_INITIALIZER(u32_to_odp(data->out_port));
>> + }
>> + return 0;
>> +}
>> +
> This looks like vxlan_push, Is there reason for having two different function?
Looking a little bigger, GRE also has pretty much the same function;
the only difference being that it calls a GRE function inside the
loop. Similarly all three pop functions are basically the same except
with a different inner function. I think we can avoid all of this by
making the netdev functions provide a loop for us, which is a little
less generic but is a little cleaner in practice. Any reason to not do
this?
>> +static void
>> netdev_vport_range(struct unixctl_conn *conn, int argc,
>> const char *argv[], void *aux OVS_UNUSED)
>> {
>> @@ -1331,7 +1441,9 @@ netdev_vport_tunnel_register(void)
>
> Otherwise looks good.
> Acked-by: Pravin B Shelar <pshe...@nicira.com>
Thanks for the reviews. I think I'll push this series with just the
FLOW_TNL_F_KEY change above and then send a follow up patch to
refactor the push/pop functions if you agree.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
