Thanks Ben,

Great to hear more from you~

Please see my reply inline,

>
> > Signed-off-by: Alex Wang <al...@nicira.com>
> >
> > ---
> > V2 -> V3:
> > - rebase.
> >
> > PATCH -> V2:
> > - explain the drop of upcall queueing priority in dpif-netdev.
> > - use mhash to calculate the 5-tuple hash.
>
> Why does dpif_netdev_recv_set() ignore its 'enable' argument?
>


I saw in current dpif-netdev.c, the dpif_netdev_recv_set() does nothing.

I didn't ask for the reason.  But now seems to be the good time, do you know
why it is not implemented?

If there is the need, I'd like follow the dpif-provider definition and
implement it.



> dp_netdev_refresh_queues() has two callers, each of which is a wrapper
> that takes the queue rwlock write-lock.  I guess that taking the lock
> could be moved into dp_netdev_refresh_queues().
>


Yes, I'll do that,



> dp_netdev_init_queue() and dp_netdev_uninit_queue() are short
> functions with only a single caller each.  I think that the code
> would be clearer if they were integrated into their callers
>

I'll do that,



> I would be more comfortable if dpif_netdev_recv() and
> dpif_netdev_recv_wait() checked that the handler_id passed in was in
> the currently valid range.
>
>
Thx, I'll do that,


> As written, flow_hash_5tuple() will incorporate ICMP type and code
> into the hash (because those are stored into tp_src and tp_dst).  Is
> that desirable?
>


I'm okay with that.  I think counting ICMP type and code in hash will not
cause
particular handler receiving unfairly more ICMP related upcalls.  (Is this
what you
concerned?)




Also, want to ask that if there is any issue with the first patch?


Thanks,
Alex Wang,
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to