Menu

#106 VxWorks TIPC does not support VLAN

closed
Missing (16)
5
2018-05-22
2009-06-02
No

Laser [gotolaser@gmail.com] has determined that VxWorks TIPC 1.7 does not support Ethernet VLAN interfaces, unlike Linux TIPC 1.7.

Laser has created a prototype patch for TIPC 1.7.6 which seems to provide this functionality (at least on VxWorks 6.3). Al has provided feedback (see below) that describes how the patch might be improved.

> - I'm not an expert in how VLAN code works in VxWorks, so I can't comment on the correctness of your analysis of the problem and your approach to solving it, but they both certainly seem to be quite reasonable.
>
> - The limitations you describe don't seem out of line with what TIPC does in other places, and are totally acceptable for an initial roll-out of VLAN support.
>
> - I don't like the way you've tried to make the "eth_bearers" array try to do "double duty" as a means to record info about TIPC bearers (both VLAN and standard ones) and Ethernet interfaces.  (It made sense to do this when there was a one-to-one correspondence between each bearer and each interface, but that's no longer true.)  I think things would be clearer if you created an "eth_interfaces" array, and put the interface-specific stuff (like the cookie returned by muxTkBind(), and a count of how many bearers were currently using the interface) in its entries.  Admittedly, this means that enabling a bearer would require searching for a free slot in each array, rather than just one, but performance isn't critical here.  On the other hard, I think you might get a performance saving in ethTipcRcvRtn() in the initial handling of untagged packets.  It might also allow you to eliminate (or simplify) some of the routines you added at the end of the file.
>
> - The way ethVlanGetBearer() is currently locating the VLAN bearer is sub-optimial, because it requires the routine to do a string comparison (of the interface name) and a numeric comparison (of the VLAN id) for every single incoming VLAN packet the parent interface receives.  (Actually, this is not quite true; you have included code that avoids this search in the case where only a single VLAN bearer is enabled on an interface, but things break down once a second VLAN bearer is used on the same interface.)  It would be more efficient to search against a single numeric "key" consisting of an interface identifier (perhaps the array index of the associated "eth_interfaces" entry I suggested?) in the high order bits and the VLAN id in the low order (12?) bits.
>
> Minor quibbles:
>
> - line 138: I don't think the tipcBoolean type is needed.  VxWorks
> defines "TRUE" and "FALSE" as 1 and 0 already, and I think it also
> defines "BOOL" as an equivalent to "int".  (I realize that your code
> makes false equivalent to -1, rather than 0, so that other changes are
> needed to your algorithms; however, the current state of affairs is
> likely to cause confusion down the road.  It certainly confused me!)
>
> - line 336: You might want to save the result of the VLAN-or-not-VLAN check so that you don't need to repeat it on line 372.
>
> - line 558: Initializing "ifName" to all zeros seems unnecessary,
> since it is never used without first being set to something else.  
> (Also, a faster way to make it an empty string is to do "ifName[0] =
> '\0';".)
>
> - line 569: I think this check can be simplified to "if (strstr(name, "vlan") == name)" to avoid the call to strlen(); alternatively, you could just put in the value 4.
>
> - line 574: You should probably return -ENODEV here, rather than -EINVAL.
>
> All things considered, this looks like an excellent start at adding VLAN support for VxWorks TIPC.  Please let me know if you have any questions about any of my comments, and feel free to send me an updated patch when you've got one.

Discussion

  • Allan Stephens

    Allan Stephens - 2009-06-02

    prototype of VxWorks TIPC 1.7.6 VLAN support

     
  • Allan Stephens

    Allan Stephens - 2009-06-02

    overview of prototype principles of operation

     
  • Jon Paul Maloy

    Jon Paul Maloy - 2018-05-22
    • Owner: Anonymous --> Jon Paul Maloy
    • Status: open --> closed
     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.