Re: [Qemu-devel] [PATCH] New feature - RFC3931 L2TPv3 network transport using static Ethernet over L2TPv3 tunnels

[Anton Ivanov]

On 05/03/14 21:22, Eric Blake wrote:

[snip]
>
> udp is layer 3, ip is layer 2 - don't you mean "optional for tcp" as the
> layer 3 counterpart of udp?

L2TPv3 uses either udp or ip as in "raw ip" (raw sockets) using a special protocol - 0x73.

So no tcp at play here. The wording is incorrect - should be "ignored for ip", thanks for noticing this.

>
>
>> +#
>> +# @dstport :#optional destination port - mandatory for udp, optional for ip
>> +#
>> +# @ipv6 :#optional - force the use of ipv6
>> +#
>> +# @udp :#optional - use the udp version of l2tpv3 encapsulation
>> +#
>> +# @cookie64:#optional - use 64 bit coookies
>
> s/:/: /
>
>> +#
>> +# @counter :#optional have sequence counter
>> +#
>> +# @txcookie :#optional 32 or 64 bit transmit cookie
>> +#
>> +# @rxcookie :#optional 32 or 64 bit receive cookie
>> +#
>> +# @txsession : 32 bit transmit session
>
> s/ :/:/
>
>> +#
>> +# @rxsession : 32 bit receive session - if not specified set to the same value as transmit
>
> Long line, please wrap to keep within 80 columns
>
>> +#
>> +# @optional : additional offset - allows the insertion of additional application-specific data before the packet payload
>
> another long line

OK

>
>> +#
>> +#
>> +# Since 1.2
>
> At this point, you've pretty much missed the 2.0 feature freeze. So the
> earliest this can be is 2.1. Specifying 1.2 is wrong either way.
>
>> +##
>> +##
>> +{ 'type': 'NetdevL2TPv3Options',
>> + 'data': {
>> + 'src': 'str',
>
> TAB damage and trailing whitespace. Please run your patch through
> scripts/checkpatch.pl, and only use spaces in the .json file.

OK.

>
>
>> + 'dst': 'str',
>> + '*srcport': 'str',
>> + '*dstport': 'str',
>> + '*ipv6': 'bool',
>> + '*udp': 'bool',
>> + '*cookie64': 'bool',
>
> Indentation looks funky - either use a single space after : in all
> cases, or align all types to the same column (I don't care which, as
> long as it isn't the mismatch of random alignments that you currently have)
>
> But looking better than the earlier version. On your next submission,
> be sure to use 'git send-email --subject-prefix=PATCHv2' to make it
> obvious that you are sending a new version.
>


--
"If you think it's expensive to hire a professional to do the job,
    wait until you hire an amateur."
                    Paul Neal "Red" Adair

A. R. Ivanov
E-mail:  address@hidden