netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@vyatta.com>
To: "Jaroslav Šafka" <jaroslav.safka@siemens.com>
Cc: netdev@vger.kernel.org
Subject: Re: bug in ip -s xfrm state list
Date: Wed, 30 May 2012 08:13:38 -0700	[thread overview]
Message-ID: <20120530081338.1904c604@nehalam.linuxnetplumber.net> (raw)
In-Reply-To: <1938601.tRGYljKHkq@notebook>

On Wed, 30 May 2012 15:00:05 +0200
Jaroslav Šafka <jaroslav.safka@siemens.com> wrote:

> Hello guys,
> I found small problem in printing "seq" number.
> I think the problem is visible in diff below ;-)
> 
> Best regards
> Jarek
> 
> diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
> index c7b3420..67fe838 100644
> --- a/ip/ipxfrm.c
> +++ b/ip/ipxfrm.c
> @@ -843,7 +843,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info 
> *xsinfo,
>         fputs(buf, fp);
>         fprintf(fp, "replay-window %u ", xsinfo->replay_window);
>         if (show_stats > 0)
> -               fprintf(fp, "seq 0x%08u ", xsinfo->seq);
> +               fprintf(fp, "seq %08u ", ntohl(xsinfo->seq));
>         if (show_stats > 0 || xsinfo->flags) {
>                 __u8 flags = xsinfo->flags;
>  
> 
> 

Moving to netdev list for more feedback.
Your change has two components:
  1. printing sequence number in decimal notation with 0x prefix.
  2. doing ntohl().

The first is an obvious bug in the original code. Not sure about
the second. It looks like sequence numbers generated by the kernel
come from xfrm_get_acqseq() which generates sequence numbers in host
byte order. But sequence numbers entered from user space (vi xfrm_seq_parse)
are encoded in network byte order. This looks like a flaw in the original design.

The kernel networking code tries to be careful about documenting network
verus host byte order via the typedef __be32 vs __u32.  Given that the kernel
is using __u32 for sequence number in xfrm fairly consistently; my opninon
is that iproute2 should change and go with the kernel practice and
use host byte order.

           reply	other threads:[~2012-05-30 15:13 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <1938601.tRGYljKHkq@notebook>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120530081338.1904c604@nehalam.linuxnetplumber.net \
    --to=shemminger@vyatta.com \
    --cc=jaroslav.safka@siemens.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).