On 2023/03/22 20:59, Simon Horman wrote:
> On Wed, Mar 15, 2023 at 03:07:22PM +0900, Nobuhiro MIKI wrote:
>> In some tunnels, inner packet needs to support both IPv4
>> and IPv6. Therefore, this patch improves to allow two
>> protocols to be tied together in one tunneling.
>>
>> Signed-off-by: Nobuhiro MIKI <nm...@yahoo-corp.jp>
>
> Hi Miki-san,
>
> in general this looks good to me.
> Some minor nits inline.
>
Hi Simon-san,
Thanks for your review.
Both comments make sense to me. I'll fix.
Best Regards,
Nobuhiro MIKI
>> ---
>> lib/tnl-ports.c | 67 +++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 45 insertions(+), 22 deletions(-)
>>
>> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
>> index 050eafa6b8c3..da9939afa8a1 100644
>> --- a/lib/tnl-ports.c
>> +++ b/lib/tnl-ports.c
>> @@ -161,40 +161,35 @@ map_insert_ipdev__(struct ip_device *ip_dev, char
>> dev_name[],
>> }
>> }
>>
>> -static uint8_t
>> -tnl_type_to_nw_proto(const char type[])
>> +static void
>> +tnl_type_to_nw_proto(const char type[], uint8_t nw_protos[2])
>> {
>> + nw_protos[0] = nw_protos[1] = 0;
>> +
>> if (!strcmp(type, "geneve")) {
>> - return IPPROTO_UDP;
>> + nw_protos[0] = IPPROTO_UDP;
>> }
>> if (!strcmp(type, "stt")) {
>
> nit: It seems slightly inefficient to call strcmp()
> again, if a match has already been made on "geneve".
> Perhaps 'else if' is more appropriate from here on down.
>
>> - return IPPROTO_TCP;
>> + nw_protos[0] = IPPROTO_TCP;
>> }
>> if (!strcmp(type, "gre") || !strcmp(type, "erspan") ||
>> !strcmp(type, "ip6erspan") || !strcmp(type, "ip6gre")) {
>> - return IPPROTO_GRE;
>> + nw_protos[0] = IPPROTO_GRE;
>> }
>> if (!strcmp(type, "vxlan")) {
>> - return IPPROTO_UDP;
>> + nw_protos[0] = IPPROTO_UDP;
>> }
>> if (!strcmp(type, "gtpu")) {
>> - return IPPROTO_UDP;
>> + nw_protos[0] = IPPROTO_UDP;
>
> nit: Not strictly related to this patch, but
> perhaps all the IPPROTO_UDP could be collapsed together,
> as is the case for IPPROTO_GRE cases.
>
>> }
>> - return 0;
>> }
>
> ...
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
