On Tue, Nov 17, 2015 at 09:26:57PM -0800, Rich Lane wrote:
> On Tue, Nov 17, 2015 at 6:56 PM, Yuanhan Liu <yuanhan.liu at linux.intel.com>
> wrote:
>
> @@ -519,6 +526,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t
> queue_id,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto merge_rx_exit;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? } else {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? update_secure_len(vq, res_cur_idx,
> &secure_len, &vec_idx);
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (secure_len == 0)
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?goto merge_rx_exit;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? res_cur_idx++;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ? ? ? ? ? } while (pkt_len > secure_len);
>
>
> I think this needs to check whether secure_len was modified. secure_len is
> read-write and could have a nonzero value going into the call. It could be
> cleaner to give update_secure_len a return value saying whether it was able to
> reserve any buffers.
Good suggestion.
--yliu
>
> Otherwise looks good, thanks!
