On Thu, Jun 19, 2014 at 2:54 PM, Jesse Gross <je...@nicira.com> wrote:
> 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.
ok.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
