Re: [Qemu-devel] [PATCH] net: L2TPv3 transport

[Anton Ivanov <anton.ivanov@kot-begemot.co.uk>]

On 10/03/14 16:06, Eric Blake wrote:
> On 03/10/2014 05:31 AM, anton.ivanov@kot-begemot.co.uk wrote:
>> From: Anton Ivanov <antivano@cisco.com>
> When sending revisions of a patch sent earlier, be sure to include
> 'PATCHv2' (or higher, as appropriate) in the subject,...

OK.

>
>> This tranport allows to connect a qemu nic to a static Ethernet
>> over L2TPv3 tunnel. The transport supports all options present
>> in the linux kernel implementation. It allows qemu to connect
>> to any linux host running kernel 3.3+, most routers and network
>>
>> Signed-off-by: Anton Ivanov <antivano@cisco.com>
>> ---
> ...and describe what changed from v1 after the --- separator.  That way,
> earlier reviewers will know what differences to look for, instead of
> assuming it is a brand new patch.

OK.

This takes into account everything discussed so far except passing 0 
instead of PROTO_L2TPV3 to getaddrinfo. That approach unfortunately does 
not work.

The .hx now includes an example versus Linux kernel which ends up being 
included in the man pages as well as the help text.

I believe we are down to 1-2 issues - the few remaining omissions you 
noticed on the interface and what to do with the socket() args and 
PROTO_L2TPV3.

>
>> +++ b/qapi-schema.json
>> @@ -2940,6 +2940,60 @@
>>       '*localaddr': 'str',
>>       '*udp':       'str' } }
>>   
>> +# @NetdevL2TPv3Options
> Missing a ## line prior to the struct name.

OK.

>
>> +# @pincounter: #optional pin sequence counter to zero - workaround for buggy implementations or networks with packet reorder
> Super long line.  Please wrap at 80 columns.

OK.
>
>> +#
>> +# @txcookie: #optional 32 or 64 bit transmit cookie
>> +#
>> +# @rxcookie: #optional 32 or 64 bit receive cookie
>> +#
>> +# @txsession: 32 bit transmit session
>> +#
>> +# @rxsession: 32 bit receive session - if not specified set to the same value as transmit
>> +#
>> +# @optional: additional offset - allows the insertion of additional application-specific data before the packet payload
> Another long line.  Also, you don't have a variable named 'optional';
> you probably meant '@offset: #optional'.

I thought I fixed that... Guess I did not...

Thanks,

A.

>