Re: [PATCH net-next 11/11] vhost_net: batch submitting XDP buffers to underlayer sockets

[Jason Wang]

On 2018å09æ08æ 00:13, Michael S. Tsirkin wrote:
> On Fri, Sep 07, 2018 at 03:41:52PM +0800, Jason Wang wrote:
> > > > @@ -556,10 +667,14 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
> > > >    	size_t len, total_len = 0;
> > > >    	int err;
> > > >    	int sent_pkts = 0;
> > > > +	bool bulking = (sock->sk->sk_sndbuf == INT_MAX);
> > > What does bulking mean?
> > The name is misleading, it means whether we can do batching. For simplicity,
> > I disable batching is sndbuf is not INT_MAX.
> But what does batching have to do with sndbuf?

If we want to do batching with sndbuf, sockets needs to return the 
number of packets that was successfully sent. And vhost need to examine 
the value.

Consider performance won't be good if sndbuf is limited, I don't 
implement this for simplicity.

> > > >    	for (;;) {
> > > >    		bool busyloop_intr = false;
> > > > +		if (nvq->done_idx == VHOST_NET_BATCH)
> > > > +			vhost_tx_batch(net, nvq, sock, &msg);
> > > > +
> > > >    		head = get_tx_bufs(net, nvq, &msg, &out, &in, &len,
> > > >    				   &busyloop_intr);
> > > >    		/* On error, stop handling until the next kick. */
> > > > @@ -577,14 +692,34 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
> > > >    			break;
> > > >    		}
> > > > -		vq->heads[nvq->done_idx].id = cpu_to_vhost32(vq, head);
> > > > -		vq->heads[nvq->done_idx].len = 0;
> > > > -
> > > >    		total_len += len;
> > > > -		if (tx_can_batch(vq, total_len))
> > > > -			msg.msg_flags |= MSG_MORE;
> > > > -		else
> > > > -			msg.msg_flags &= ~MSG_MORE;
> > > > +
> > > > +		/* For simplicity, TX batching is only enabled if
> > > > +		 * sndbuf is unlimited.
> > > What if sndbuf changes while this processing is going on?
> > We will get the correct sndbuf in the next run of handle_tx(). I think this
> > is safe.
> If it's safe why bother with special-casing INT_MAX?

The difference is handle_tx() won't loop forever and will recognize the 
new value next time, we have a quota to limit this.

Thanks