On Tue, Nov 08, 2011 at 03:57:29PM -0800, Justin Pettit wrote:
> This will be useful later when we add support for matching the ECN bits
> within the TOS field.
>
> Signed-off-by: Justin Pettit <[email protected]>
I think that these |=s can become =s:
> if (offset) {
> - key->ip.tos_frag |= OVS_FRAG_TYPE_LATER;
> + key->ip.frag |= OVS_FRAG_TYPE_LATER;
> goto out;
> }
> if (nh->frag_off & htons(IP_MF) ||
> skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> - key->ip.tos_frag |= OVS_FRAG_TYPE_FIRST;
> + key->ip.frag |= OVS_FRAG_TYPE_FIRST;
>
> /* Transport layer. */
> if (key->ip.proto == IPPROTO_TCP) {
> @@ -764,10 +762,10 @@ int flow_extract(struct sk_buff *skb, u16 in_port,
> struct sw_flow_key *key,
> goto out;
> }
>
> - if ((key->ip.tos_frag & OVS_FRAG_TYPE_MASK) ==
> OVS_FRAG_TYPE_LATER)
> + if (key->ip.frag == OVS_FRAG_TYPE_LATER)
> goto out;
> if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> - key->ip.tos_frag |= OVS_FRAG_TYPE_FIRST;
> + key->ip.frag |= OVS_FRAG_TYPE_FIRST;
>
> /* Transport layer. */
> if (key->ip.proto == NEXTHDR_TCP) {
Did you mean to update the comment on 'frag' below?
> struct sw_flow_key {
> struct {
> __be64 tun_id; /* Encapsulating tunnel ID. */
> @@ -48,8 +44,8 @@ struct sw_flow_key {
> } eth;
> struct {
> u8 proto; /* IP protocol or lower 8 bits of ARP
> opcode. */
> - u8 tos_frag; /* IP ToS DSCP in high 6 bits,
> - * OVS_FRAG_TYPE_* in low 2 bits. */
> + u8 tos; /* IP ToS DSCP in high 6 bits. */
> + u8 frag; /* OVS_FRAG_TYPE_* in low 2 bits. */
> } ip;
> union {
> struct {
In struct, could we put the 'frag' byte after arp_tha so that arp_sha
and arp_tha don't start on odd addresses:
> @@ -70,21 +70,23 @@ struct flow {
> uint8_t dl_src[6]; /* Ethernet source address. */
> uint8_t dl_dst[6]; /* Ethernet destination address. */
> uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode.
> */
> - uint8_t tos_frag; /* IP ToS in top bits, FLOW_FRAG_* in low. */
> + uint8_t tos; /* IP ToS. */
> + uint8_t frag; /* FLOW_FRAG_*. */
> uint8_t arp_sha[6]; /* ARP/ND source hardware address. */
> uint8_t arp_tha[6]; /* ARP/ND target hardware address. */
> + uint8_t reserved[7]; /* Reserved for 64-bit packing. */
> };
In nx-match.c, I think that it might be a little better to keep the
nxm_put_tos_frag() function together just to reduce code duplication.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
