On Thu, Jun 19, 2014 at 2:01 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Thu, Jun 19, 2014 at 1:46 PM, Jesse Gross <je...@nicira.com> wrote:
>> On Thu, Jun 19, 2014 at 1:30 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>>> On Tue, Jun 10, 2014 at 4:47 PM, Jesse Gross <je...@nicira.com> wrote:
>>>> @@ -520,18 +534,24 @@ int ovs_flow_extract(struct sk_buff *skb, u16
>>>> in_port, struct sw_flow_key *key)
>>>> key->tp.src = tcp->source;
>>>> key->tp.dst = tcp->dest;
>>>> key->tp.flags = TCP_FLAGS_BE16(tcp);
>>>> + } else {
>>>> + memset(&key->tp, 0, sizeof(key->tp));
>>>> }
>>>> } else if (key->ip.proto == IPPROTO_UDP) {
>>>> if (udphdr_ok(skb)) {
>>>> struct udphdr *udp = udp_hdr(skb);
>>>> key->tp.src = udp->source;
>>>> key->tp.dst = udp->dest;
>>>> + } else {
>>>> + memset(&key->tp, 0, sizeof(key->tp));
>>>> }
>>> if we combine above if condition we can have common memset for key->tp.
>>
>> I'm not sure I understand what you mean here - each protocol has a
>> different check for the header being present. What would a combined
>> version look like?
>
> I was thinking of something like:
>
> if (key->ip.proto == IPPROTO_TCP && tcphdr_ok(skb)) {
> struct tcphdr *tcp = tcp_hdr(skb);
> key->tp.src = tcp->source;
> key->tp.dst = tcp->dest;
> key->tp.flags = TCP_FLAGS_BE16(tcp);
> } else if (key->ip.proto == IPPROTO_UDP &&udphdr_ok(skb)) {
> .....
> .....
> }else if (...) {
> .....
> } else {
> memset(&key->tp, 0, sizeof(key->tp));
> }
OK, I see what you mean now. It does save some code but I guess it
seems a little less obvious to me what is going on. This would change
the condition for clearing the fields from there being an error to not
having a valid protocol in the list. In practice this is essentially
the same but logically it's a little different.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
