On 19/04/2016 13:03, "Joe Stringer" <j...@ovn.org> wrote:

>On 15 April 2016 at 17:02, Daniele Di Proietto <diproiet...@vmware.com> wrote:
>> This will be used by a future commit.
>>
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> ---
>>  lib/flow.c | 140 
>> ++++++++++++++++++++++++++++++++++---------------------------
>>  lib/flow.h |   3 ++
>>  2 files changed, 81 insertions(+), 62 deletions(-)
>>
>> diff --git a/lib/flow.c b/lib/flow.c
>> index 560a90f..972a996 100644
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -439,6 +439,82 @@ invalid:
>>      arp_buf[1] = eth_addr_zero;
>>  }
>>
>> +static inline bool
>> +parse_ipv6_ext_hdrs__(const void **datap, size_t *sizep, uint8_t *nw_proto,
>> +                      uint8_t *nw_frag)
>> +{
>> +    while (1) {
>> +        if (OVS_LIKELY((*nw_proto != IPPROTO_HOPOPTS)
>> +                       && (*nw_proto != IPPROTO_ROUTING)
>> +                       && (*nw_proto != IPPROTO_DSTOPTS)
>> +                       && (*nw_proto != IPPROTO_AH)
>> +                       && (*nw_proto != IPPROTO_FRAGMENT))) {
>> +            /* It's either a terminal header (e.g., TCP, UDP) or one we
>> +             * don't understand.  In either case, we're done with the
>> +             * packet, so use it to fill in 'nw_proto'. */
>> +            return true;
>> +        }
>> +
>> +        /* We only verify that at least 8 bytes of the next header are
>> +         * available, but many of these headers are longer.  Ensure that
>> +         * accesses within the extension header are within those first 8
>> +         * bytes. All extension headers are required to be at least 8
>> +         * bytes. */
>> +        if (OVS_UNLIKELY(*sizep < 8)) {
>> +            return false;
>> +        }
>> +
>> +        if ((*nw_proto == IPPROTO_HOPOPTS)
>> +            || (*nw_proto == IPPROTO_ROUTING)
>> +            || (*nw_proto == IPPROTO_DSTOPTS)) {
>> +            /* These headers, while different, have the fields we care
>> +             * about in the same location and with the same
>> +             * interpretation. */
>> +            const struct ip6_ext *ext_hdr = *datap;
>> +            *nw_proto = ext_hdr->ip6e_nxt;
>> +            if (OVS_UNLIKELY(!data_try_pull(datap, sizep,
>> +                                            (ext_hdr->ip6e_len + 1) * 8))) {
>> +                return false;
>> +            }
>> +        } else if (*nw_proto == IPPROTO_AH) {
>> +            /* A standard AH definition isn't available, but the fields
>> +             * we care about are in the same location as the generic
>> +             * option header--only the header length is calculated
>> +             * differently. */
>> +            const struct ip6_ext *ext_hdr = *datap;
>> +            *nw_proto = ext_hdr->ip6e_nxt;
>> +            if (OVS_UNLIKELY(!data_try_pull(datap, sizep,
>> +                                            (ext_hdr->ip6e_len + 2) * 4))) {
>> +                return false;
>> +            }
>> +        } else if (*nw_proto == IPPROTO_FRAGMENT) {
>> +            const struct ovs_16aligned_ip6_frag *frag_hdr = *datap;
>> +
>> +            *nw_proto = frag_hdr->ip6f_nxt;
>> +            if (!data_try_pull(datap, sizep, sizeof *frag_hdr)) {
>> +                return false;
>> +            }
>> +
>> +            /* We only process the first fragment. */
>> +            if (frag_hdr->ip6f_offlg != htons(0)) {
>> +                *nw_frag = FLOW_NW_FRAG_ANY;
>> +                if ((frag_hdr->ip6f_offlg & IP6F_OFF_MASK) != htons(0)) {
>> +                    *nw_frag |= FLOW_NW_FRAG_LATER;
>> +                    *nw_proto = IPPROTO_FRAGMENT;
>> +                    return true;
>> +                }
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +bool
>> +parse_ipv6_ext_hdrs(const void **datap, size_t *sizep, uint8_t *nw_proto,
>> +                    uint8_t *nw_frag)
>> +{
>> +    return parse_ipv6_ext_hdrs__(datap, sizep, nw_proto, nw_frag);
>> +}
>> +
>
>I couldn't tell any difference between parse_ipv6_ext_hdrs() and
>parse_ipv6_ext_hdrs__(); are they both necessary?
>
>Acked-by: Joe Stringer <j...@ovn.org>

You're right, there's no difference, but miniflow_extract() can call
the inline version, which usually generates better code.

Thanks for the review
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to