On 17-01-12 02:22 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 12, 2017 at 01:45:19PM -0800, John Fastabend wrote:
>> Add support for XDP adjust head by allocating a 256B header region
>> that XDP programs can grow into. This is only enabled when a XDP
>> program is loaded.
>>
>> In order to ensure that we do not have to unwind queue headroom push
>> queue setup below bpf_prog_add. It reads better to do a prog ref
>> unwind vs another queue setup call.
>>
>> At the moment this code must do a full reset to ensure old buffers
>> without headroom on program add or with headroom on program removal
>> are not used incorrectly in the datapath. Ideally we would only
>> have to disable/enable the RX queues being updated but there is no
>> API to do this at the moment in virtio so use the big hammer. In
>> practice it is likely not that big of a problem as this will only
>> happen when XDP is enabled/disabled changing programs does not
>> require the reset. There is some risk that the driver may either
>> have an allocation failure or for some reason fail to correctly
>> negotiate with the underlying backend in this case the driver will
>> be left uninitialized. I have not seen this ever happen on my test
>> systems and for what its worth this same failure case can occur
>> from probe and other contexts in virtio framework.
>
> Could you explain about this a bit more?
> Thanks!
>
Sure. There are two existing paths and this patch adds a third one
where the driver basically goes through this reset path. First one
is on probe the other one is on the freeze/restore path.
The virtnet_freeze() path eventually free's the memory for rq/sq
(receive queues and send queues).
virtnet_freeze()
...
remove_vq_common()
...
virtnet_dev_vqs()
vdev->config->del_vqs()
virtnet_free_queues <- this does a kfree
On virtnet_restore() path we then have to reallocate and reneg with
backend.
virtnet_retore()
...
init_vqs()
...
virtnet_alloc_queues() <- alloc sq/rq
virtnet_find_vqs()
(allocates callbacks/names/vqs)
So the above allocs could fail and leave the device in a FAILED
state. This can happen today on probe or freeze/restore paths and
after this patch possibly on XDP load. Although as noted I have not
seen it happen in any of the above cases.
Second failure mode could happen if virtio_finalize_features() fails.
This seems unlikely because in order to probe successfully we had to
finalize the features successfully earlier. But it could I guess happen
based on return codes. Again never seen this actually happen. This is
called in probe case, freeze/restore case, and XDP now as well.
Does that help? Also I need to send a v2 to fix a spelling mistake and
to convert a 'unsigned' to 'unsigned int' per checkpatch warning. Always
better to run checkpatch before submitting vs after.
Thanks,
John
