Re: [PATCH] 802.1p priority (fixed)

From: Brooks Davis (brooks_at_one-eyed-alien.net)
Date: 01/25/05

  • Next message: Anne Marcel Roorda: "Re: public ip address behind nat" 
    Date: Mon, 24 Jan 2005 22:29:14 -0800
To: Boris Kovalenko <boris@ntmk.ru>

    On Tue, Jan 25, 2005 at 09:16:02AM +0500, Boris Kovalenko wrote:
    > Hello!
    >
    > >by this specific implementation. I'm sure we can keep an interface that
    > >handles priorities as seperate interfaces, but I'm not sure we'll want
    > >to do it via the vlan device (attractivly simple though that is.)
    > >
    > >This patch appears to be against 4 or 5. In 6 we've largly rewritten
    > >ifconfig so the patch won't apply. It looks like a simple matter to fix
    > >this issue. We'll need to commit to 6 before 4 or 5.
    > >
    > >I've embeded some comments in the text below.
    > Ok, so what I should do now? Rewrite patch for 6?

    Let's get the version for 5 looking good first since we should be able
    to MFC. We can do the ifconfig part for 6 once that's clean.

    > >>+ if(tag < 1 || tag > 4094)
    > >>+ errx(1, "VLAN ID shoud be in range 1..4094");
    > >
    > >
    > >errx should be fully indented.
    > What this means? What difference between my errx and this one (from 6)?
    > errx(1, "must specify both vlan tag and device");

    err is indented with a tab and four spaces. It should be indented with
    two tabs like this:

            if(tag < 1 || tag > 4094)
                    errx(1, "VLAN ID shoud be in range 1..4094");

    > >I know other nearby code does this, but atoi should not be used. It has
    > >not useful error checking. strtoul should be used instead.
    > No problem.
    > >> */
    > >>struct vlanreq {
    > >>- char vlr_parent[IFNAMSIZ];
    > >>- u_short vlr_tag;
    > >>+ char vlr_parent[IFNAMSIZ];
    > >>+ u_int16_t vlr_tag;
    > >
    > >
    > >This appears to be a no-op. Is it needed?
    > Hmm... just to clarify that vlr_tag is 16bit value. If this is
    > unnecessary I may use u_short.

    Assuming the code compiles cleanly, let's leave it a u_short in 5. We
    can change it to a u_int16_t in 6 since I agree that's more precise.
    Changes to types, even ones that obviously don't do anything tend to
    make re@ concerned about possible ABI breakage so I'd rather not worry
    them. I tend to do that enough with my other projects. :)

    -- Brooks 

    -- 
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529  9BF0 5D8E 8BE9 F238 1AD4

  • Next message: Anne Marcel Roorda: "Re: public ip address behind nat"