Sounds good, thanks. Ethan
On Wed, Apr 18, 2012 at 09:36, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Apr 17, 2012 at 05:21:03PM -0700, Ethan Jackson wrote: >> Does nl_sock_allocate_seq() need to take 'n' as a parameter? It's >> only caller passes in 1 currently. Perhaps a future patch will need >> it? >> >> It's probably worth adding a comment to nl_sock_allocate_seq() >> explaining why it wraps around at (UINT32_MAX / 2) without a deep >> understanding of netlink, it's not obvious to me why it's necessary. >> >> Does the theoretical race condition described in the old >> get_nlmsg_seq() function still exist? Just curious, may not be a big >> deal. If it does, maybe it makes sense to use a random initial >> sequence number instead of 1? Wouldn't solve the problem, but might >> reduce the probability of a collision. > > Good points. > > I added a comment to handle the nl_sock_allocate_seq() for your second > point: > > /* Make it impossible for the next request for sequence numbers to wrap > * around to 0. Start over with 1 to avoid ever using a sequence number of > * 0, because the kernel uses sequence number 0 for notifications. */ > > I added the following to the commit message to address your other points: > > Choosing sequence numbers at time of creating a packet means that > nl_sock_transact_multiple() has to search for the sequence number > of a reply, because the sequence numbers of the requests aren't > necessarily sequential. This commit makes it possible to avoid > the search, by deferring choice of sequence numbers until the > time that we send the packets. It doesn't actually modify > nl_sock_transact_multiple(), which will happen in a later commit. > > Previously, I was concerned about a theoretical race condition > described in a comment in the old versino of this code: > > This implementation uses sequence numbers that are unique > process-wide, to avoid a hypothetical race: send request, close > socket, open new socket that reuses the old socket's PID value, > send request on new socket, receive reply from kernel to old > socket but with same PID and sequence number. (This race could be > avoided other ways, e.g. by preventing PIDs from being quickly > reused). > > However, I no longer believe that this can be a real problem, > because Netlink operates synchronously. The reply to a request > will always arrive before the socket can be closed and a new > socket opened with the old socket's PID. > > Thanks, > > Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
