On Mon, Feb 18, 2013 at 4:45 AM, Lori Jakab <loja...@cisco.com> wrote:
> On 02/18/13 14:31, Lori Jakab wrote:
>> On 02/16/13 00:45, Jesse Gross wrote:
>>> On Fri, Feb 15, 2013 at 10:34 AM, Lori Jakab <loja...@cisco.com> wrote:
>>>> On 02/15/13 01:32, Jesse Gross wrote:
>>>>> On Wed, Feb 13, 2013 at 6:44 AM, Lorand Jakab <loja...@cisco.com> wrote:
>>>>>> +/* Compute source port for outgoing packet.
>>>>>> + * Currently we use the flow hash.
>>>>>> + */
>>>>>> +static u16 get_src_port(struct sk_buff *skb)
>>>>>> +{
>>>>>> + int low;
>>>>>> + int high;
>>>>>> + unsigned int range;
>>>>>> + u32 hash = OVS_CB(skb)->flow->hash;
>>>>>> +
>>>>>> + inet_get_local_port_range(&low, &high);
>>>>>> + range = (high - low) + 1;
>>>>>> + return (((u64) hash * range) >> 32) + low;
>>>>>> +}
>>>>>
>>>>> Since this is the same as VXLAN, it might be worth factoring it out to
>>>>> some common location.
>>>>
>>>> What would be the correct place for this? datapath/tunnel.c ? Or maybe
>>>> as a 'static inline' function in datapath/tunnel.h ? I will move it
>>>> there and remove it from the VXLAN vport code as well.
>>>
>>> Probably datapath/tunnel.h makes the most sense.
>>
>> OK. There is one issue with this though. datapath/datapath.h includes
>> datapath/tunnel.h, but the OVS_CB macro, required by the the new
>> function, is defined in the former. I can either expand the macro in
>> place of OVS_CB in tunnel.h (a bit ugly) or move the #define from
>> datapath.h to tunnel.h. Which solution would you prefer?
>
> Sorry, I hit send too soon, I just realized that the structure which is
> used for the cast in the macro is defined there as well. Since
> datapath.h depends on tunnel.h, I suggest we move the get_src_port()
> function into tunnel.c instead of tunnel.h.
Moving get_src_port() into tunnel.c is fine and is definitely the
cleanest solution of the ones you listed.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
