[ovs-dev] [optimize 08/13] netlink-socket: New function nl_sock_transact_multiple().
Ben Pfaff
blp at nicira.com
Fri Oct 14 20:51:30 UTC 2011
On Thu, Oct 13, 2011 at 01:23:54PM -0700, Ethan Jackson wrote:
> I don't know a great deal about IOV maximums so the next comment may not be
> reasonable:
> I found _XOPEN_IOV_MAX a surprising minimum. Wouldn't sysconf's opinion of
> max_iovs be less than _XOPEN_IOV_MAX in rare error conditions?
_XOPEN_IOV_MAX (which is always defined as 16) is the minimum value
that POSIX allows for IOV_MAX, so a system that supports fewer iovecs
than that is nonconformant.
> In this case wouldn't 1 be a safer minimum choice?
A kernel that supports only 1 iovec isn't very useful. Anyway Linux
supports up to 1024 iovecs and this is a Linux-specific file, so maybe
I shouldn't have bothered with all this standards crap.
> + } else if (max_iovs > MAX_IOVS) {
> + max_iovs = 128;
>
>
> I think you want max_iovs = MAX_IOVS.
Yes, thanks.
>
> + max_batch_count = MAX(sock->rcvbuf / 4096, 1);
> + max_batch_count = MIN(max_batch_count, MAX_IOVS / 2);
>
>
> Did you choose 4096 here because it's the typical page size? Would it make
> sense to use the PAGESIZE macro if defined?
No, I chose it as a reasonable compromise between the maximum reply
size of 64 kB and the common reply size (about 0 bytes) that allows a
reasonable amount of batching with the typical rcvbuf size (about 128
kB)--see the comment.
> + if (error == ENOBUFS) {
> + VLOG_DBG_RL(&rl, "receive buffer overflow, resending request");
>
>
> Is this code supposed to resend the request itself, or is the caller
> responsible for handling that?
It automatically resends it in the next loop iteration.
> + } else if (error) {
> + VLOG_ERR_RL(&rl, "transaction error (%s)", strerror(error));
> + nl_sock_record_errors__(transactions, n, error);
> + }
> + }
> +}
>
> We may want to undef MAX_BATCH_BYTES. Probably doesn't matter, but seems
> stylistically cleaner.
I changed it to an enum, thanks.
More information about the dev
mailing list