On Tue, Apr 28, 2015 at 4:28 PM, Jesse Gross <je...@nicira.com> wrote:
> On Mon, Apr 27, 2015 at 2:04 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>> diff --git a/datapath/linux/compat/include/net/stt.h
>> b/datapath/linux/compat/include/net/stt.h
>> new file mode 100644
>> index 0000000..96181fd
>> --- /dev/null
>> +++ b/datapath/linux/compat/include/net/stt.h
>> +#define stt_sock_add rpl_stt_sock_add
>> +struct stt_sock *rpl_stt_sock_add(struct net *net, __be16 port,
>> + stt_rcv_t *rcv, void *data);
>> +
>> +#define stt_sock_release rpl_stt_sock_release
>> +void rpl_stt_sock_release(struct stt_sock *stt_sock);
>> +
>> +#define stt_xmit_skb rpl_stt_xmit_skb
>> +int rpl_stt_xmit_skb(struct sk_buff *skb, struct rtable *rt,
>> + __be32 src, __be32 dst, __u8 tos,
>> + __u8 ttl, __be16 df, __be16 src_port, __be16 dst_port,
>> + __be64 tun_id);
>> +
>> +#define stt_init_module rpl_stt_init_module
>> +int rpl_stt_init_module(void);
>> +
>> +#define stt_cleanup_module rpl_stt_cleanup_module
>> +void rpl_stt_cleanup_module(void);
>
> I think for stt_init/cleanup_module(), these should probably be
> prefixed with ovs_ instead of rpl_ and not use a macro since they are
> pure compatibility functions and wouldn't be used in a theoretical
> upstream version.
>
ok, I changed prefix to ovs.
> The other three functions are more debatable since they would be
> upstream but aren't in practice, so I could see arguments for it
> either way.
>
for now I kept it as is.
>> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
>> new file mode 100644
>> index 0000000..aff4751
>> --- /dev/null
>> +++ b/datapath/linux/compat/stt.c
>> +struct first_frag {
>> + struct sk_buff *last_skb;
>> + unsigned int mem_used;
>> + u16 tot_len;
>> + u16 rcvd_len;
>> + bool ecn_ce;
>> +};
>
> I just noticed that the ecn_ce field appears to be write only. We
> should set this on the outer IP header of the reassembled packet if
> any of the individual segments have it set.
>
ok I fixed it.
> These are pretty minor changes though and I am otherwise happy with
> this. Thanks for all of the revisions.
>
> Acked-by: Jesse Gross <je...@nicira.com>
I made the changes you asked above. I will push the patch shortly.
Thanks for detailed review.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
