[ovs-dev] [PATCH 1/2] datapath: Rearrange struct sw_flow_key to put optional information last.

Andrew Evans aevans at nicira.com
Tue Apr 26 23:53:00 UTC 2011


On 4/26/11 3:02 PM, Jesse Gross wrote:
> On Mon, Apr 25, 2011 at 6:11 PM, Andrew Evans <aevans at nicira.com> wrote:
>> IPv4/IPv6-specific fields are currently scattered about struct
>> sw_flow_table. Group them together in a union at the end to make it possible to
> 
> This should be struct sw_flow_key, right?

Yes, thank you. Fixed.

>> +                       union {
>> +                               struct {
>> +                                       __be16 src; /* TCP/UDP source port. */
>> +                                       __be16 dst; /* TCP/UDP destination port.
>> +                                                    */
> 
> I don't think you need to be quite so aggressive about line wrapping.
> Here I think it decreases readability.

Ok. I've unwrapped the rest of the comments in this struct too.

>> +                               } tp;
>> +                               struct in6_addr nd_target; /* ND target address.
>> +                                                           */
>> +                       };
> 
> I don't think that these can be in a union together because IPv6
> neighbor discovery packets are ICMP messages so they have both a
> type/code and the ND addresses (unlike IPv4).

Oops, right you are. De-unionized.

>> +                       u8 nd_sha[ETH_ALEN]; /* ND source hardware address. */
>> +                       u8 nd_tha[ETH_ALEN]; /* ND target hardware address. */
> 
> IPv6 calls these SLL/TLL (source/target link layer) so we should
> probably use the correct names now that they are broken out.

Ok. I'm inclined to put all the neighbor-discovery-related stuff into a
struct, i.e.:

...
struct {
	struct in6_addr target; /* ND target address. */
	u8 sll[ETH_ALEN]; /* ND source hardware address. */
	u8 tll[ETH_ALEN]; /* ND target hardware address. */
} nd;
...

Does that seem like a good idea to you?



More information about the dev mailing list