a mistake in nova/utils.py/vpn_ping

[Can Zhang]

I'm just starting to read the source code of nova, but I believe someone has made a mistake.
Because the comments and code don't match

the comments:
    Client packet (14 bytes)::
     0 1 8 9 13
    +-+--------+-----+
    |x| cli_id |?????|
    +-+--------+-----+
    x = packet identifier 0x38
    cli_id = 64 bit identifier
    ? = unknown, probably flags/padding

the code:

    data = struct.pack('!BQxxxxxx', 0x38, session_id)

where in the format string:
       ! for network(=big-endian)
       B for unsigned char, 1 byte
       Q for unsigned long long, 8 byte
       x for pad byte

I think the format string !BQxxxxxx should be !BQxxxxx

btw, I actually don't understand what this function do and the name 'vpn_ping' confuses me
can anyone explain it?

[Vish Ishaya (vishvananda)]

The code sends a fake handshake packet to a vpn server and waits for a valid repsonse. It allows us to programmatically check if a vpn is listening as it should.

On Dec 19, 2011, at 6:43 AM, can. wrote:

> Public bug reported:
>
> I'm just starting to read the source code of nova, but I believe someone has made a mistake.
> Because the comments and code don't match
>
> the comments:
> Client packet (14 bytes)::
> 0 1 8 9 13
> +-+--------+-----+
> |x| cli_id |?????|
> +-+--------+-----+
> x = packet identifier 0x38
> cli_id = 64 bit identifier
> ? = unknown, probably flags/padding
>
> the code:
>
> data = struct.pack('!BQxxxxxx', 0x38, session_id)
>
> where in the format string:
> ! for network(=big-endian)
> B for unsigned char, 1 byte
> Q for unsigned long long, 8 byte
> x for pad byte
>
> I think the format string !BQxxxxxx should be !BQxxxxx
>
> btw, I actually don't understand what this function do and the name 'vpn_ping' confuses me
> can anyone explain it?
>
> ** Affects: nova
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to
> OpenStack Compute (nova).
> https://bugs.launchpad.net/bugs/906346
>
> Title:
> a mistake in nova/utils.py/vpn_ping
>
> Status in OpenStack Compute (Nova):
> New
>
> Bug description:
> I'm just starting to read the source code of nova, but I believe someone has made a mistake.
> Because the comments and code don't match
>
> the comments:
> Client packet (14 bytes)::
> 0 1 8 9 13
> +-+--------+-----+
> |x| cli_id |?????|
> +-+--------+-----+
> x = packet identifier 0x38
> cli_id = 64 bit identifier
> ? = unknown, probably flags/padding
>
> the code:
>
> data = struct.pack('!BQxxxxxx', 0x38, session_id)
>
> where in the format string:
> ! for network(=big-endian)
> B for unsigned char, 1 byte
> Q for unsigned long long, 8 byte
> x for pad byte
>
> I think the format string !BQxxxxxx should be !BQxxxxx
>
> btw, I actually don't understand what this function do and the name 'vpn_ping' confuses me
> can anyone explain it?
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nova/+bug/906346/+subscriptions

[Can Zhang (acme-ican)]

so the format string is written wrong deliberately?

[Thierry Carrez (ttx)]

No, the bug remains -- Vish was just answering your question about what the code is used for.

[Jesse Andrews (anotherjesse)]

I think dean troyer did some of the original work - adding him as a reviewer

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/3600
Committed: http://github.com/openstack/nova/commit/014f67a2769720f4833bba74343cedf0e894151c
Submitter: Jenkins
Branch: master

commit 014f67a2769720f4833bba74343cedf0e894151c
Author: Russell Bryant <email address hidden>
Date: Tue Jan 31 17:42:23 2012 -0500

    Fix VPN ping packet length.

    Fix bug 906346.

    This patch addresses the typo pointed out in bug 906346. The ping being
    sent was 15 bytes long when it should have been 14. Removing a pad byte
    from the format string resolves this issue. I verified that the format
    described in the code comments (and now the code) was correct using an
    OpenVPN connection setup packet capture.

    Change-Id: Idbc5e48ede4a8d2836dd1b102a9a0e172540776c