OpenStack Compute (Nova)

a mistake in nova/utils.py/vpn_ping

Reported by can. on 2011-12-19
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Low
Russell Bryant

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?

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. (acme-ican) wrote :

so the format string is written wrong deliberately?

Thierry Carrez (ttx) wrote :

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

Changed in nova:
importance: Undecided → Low
status: New → Triaged
Jesse Andrews (anotherjesse) wrote :

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

Changed in nova:
assignee: nobody → Russell Bryant (russellb)
status: Triaged → In Progress

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

Changed in nova:
status: In Progress → Fix Committed
Changed in nova:
milestone: none → essex-4
Thierry Carrez (ttx) on 2012-02-29
Changed in nova:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2012-04-05
Changed in nova:
milestone: essex-4 → 2012.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers