* Re: [virtio-dev] Re: [PATCH v1] virtio: support VIRTIO_F_IO_BARRIER
From: Stefan Hajnoczi @ 2018-05-10 15:02 UTC (permalink / raw)
To: Tiwei Bie
Cc: virtio-dev, mst, linux-kernel, virtualization, zhihong.wang,
pbonzini
In-Reply-To: <20180510103941.znxka65iluybktuq@debian>
[-- Attachment #1.1: Type: text/plain, Size: 1269 bytes --]
On Thu, May 10, 2018 at 06:39:41PM +0800, Tiwei Bie wrote:
> On Thu, May 10, 2018 at 10:53:17AM +0100, Stefan Hajnoczi wrote:
> > On Fri, May 04, 2018 at 12:59:15PM +0800, Tiwei Bie wrote:
> > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > index 308e2096291f..9fb519a9df28 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -49,7 +49,7 @@
> > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > * bits. */
> > > #define VIRTIO_TRANSPORT_F_START 28
> > > -#define VIRTIO_TRANSPORT_F_END 34
> > > +#define VIRTIO_TRANSPORT_F_END 37
> >
> > Have you updated "2.2 Feature Bits" in the VIRTIO spec?
> >
> > In 1.0 it says:
> >
> > 24 to 32
> > Feature bits reserved for extensions to the queue and feature negotiation mechanisms
> >
> > This information is out-of-date.
> >
>
> No. In the latest spec draft, it's 24 to 33. And it
> becomes out-of-date since VIRTIO_F_RING_PACKED(34)
> and VIRTIO_F_IN_ORDER(35) were introduced. Do you
> want me to update it in the IO_BARRIER patch or do
> you want me to update it in a new patch?
Please update it to 37 in the IO_BARRIER patch.
Stefan
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-05-10 10:50 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, wexu, virtualization, linux-kernel, mst
In-Reply-To: <2fc35cd5-9dbd-7743-497f-b6637d92f528@redhat.com>
On Thu, May 10, 2018 at 05:49:20PM +0800, Jason Wang wrote:
> On 2018年05月10日 16:56, Tiwei Bie wrote:
> > On Thu, May 10, 2018 at 03:34:50PM +0800, Jason Wang wrote:
> > > On 2018年05月10日 15:32, Jason Wang wrote:
> > > > On 2018年04月25日 13:15, Tiwei Bie wrote:
> > > > > + /* We're using some buffers from the free list. */
> > > > > + vq->vq.num_free -= descs_used;
> > > > > +
> > > > > + /* Update free pointer */
> > > > > + if (indirect) {
> > > > > + n = head + 1;
> > > > > + if (n >= vq->vring_packed.num) {
> > > > > + n = 0;
> > > > > + vq->wrap_counter ^= 1;
> > > > > + }
> > > > > + vq->next_avail_idx = n;
> > > > > + } else
> > > > > + vq->next_avail_idx = i;
> > > > During testing zerocopy (out of order completion), I found driver may
> > > > submit two identical buffer id to vhost. So the above code may not work
> > > > well.
> > > >
> > > > Consider the case that driver adds 3 buffer and virtqueue size is 8.
> > > >
> > > > a) id = 0,count = 2,next_avail = 2
> > > >
> > > > b) id = 2,count = 4,next_avail = 2
> > > next_avail should be 6 here.
> > >
> > > > c) id = 4,count = 2,next_avail = 0
> > > >
> > > id should be 6 here.
> > >
> > > Thanks
> > >
> > > > if packet b is done before packet a, driver may think buffer id 0 is
> > > > available and try to use it if even if the real buffer 0 was not done.
> > > >
> > > > Thanks
> > Nice catch! Thanks a lot!
> > I'll implement an ID allocator.
> >
> > Best regards,
> > Tiwei Bie
>
> Sounds good.
>
> Another similar issue is detac_buf_packed(). It did:
>
> for (j = 0; j < vq->desc_state[head].num; j++) {
> desc = &vq->vring_packed.desc[i];
> vring_unmap_one_packed(vq, desc);
> i++;
> if (i >= vq->vring_packed.num)
> i = 0;
> }
>
> This probably won't work for out of order too and according to the spec:
>
> """
> Driver needs to keep track of the size of the list corresponding to each
> buffer ID, to be able to skip to where the next used descriptor is written
> by the device.
> """
>
> Looks like we should not depend on the descriptor ring.
Yeah, the previous ID allocation is too simple..
Let me fix it in the next version.
Thanks!
>
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v1] virtio: support VIRTIO_F_IO_BARRIER
From: Tiwei Bie @ 2018-05-10 10:39 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: virtio-dev, mst, linux-kernel, virtualization, zhihong.wang,
pbonzini
In-Reply-To: <20180510095317.GO1296@stefanha-x1.localdomain>
On Thu, May 10, 2018 at 10:53:17AM +0100, Stefan Hajnoczi wrote:
> On Fri, May 04, 2018 at 12:59:15PM +0800, Tiwei Bie wrote:
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..9fb519a9df28 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 37
>
> Have you updated "2.2 Feature Bits" in the VIRTIO spec?
>
> In 1.0 it says:
>
> 24 to 32
> Feature bits reserved for extensions to the queue and feature negotiation mechanisms
>
> This information is out-of-date.
>
No. In the latest spec draft, it's 24 to 33. And it
becomes out-of-date since VIRTIO_F_RING_PACKED(34)
and VIRTIO_F_IN_ORDER(35) were introduced. Do you
want me to update it in the IO_BARRIER patch or do
you want me to update it in a new patch?
Best regards,
Tiwei Bie
^ permalink raw reply
* Re: [virtio-dev] [PATCH v1] virtio: support VIRTIO_F_IO_BARRIER
From: Stefan Hajnoczi @ 2018-05-10 9:53 UTC (permalink / raw)
To: Tiwei Bie
Cc: virtio-dev, mst, linux-kernel, virtualization, zhihong.wang,
pbonzini
In-Reply-To: <20180504045915.17693-1-tiwei.bie@intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 718 bytes --]
On Fri, May 04, 2018 at 12:59:15PM +0800, Tiwei Bie wrote:
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..9fb519a9df28 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 37
Have you updated "2.2 Feature Bits" in the VIRTIO spec?
In 1.0 it says:
24 to 32
Feature bits reserved for extensions to the queue and feature negotiation mechanisms
This information is out-of-date.
Stefan
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Jason Wang @ 2018-05-10 9:49 UTC (permalink / raw)
To: Tiwei Bie; +Cc: netdev, wexu, virtualization, linux-kernel, mst
In-Reply-To: <20180510085601.6mpxf3yvwxnqnk5q@debian>
On 2018年05月10日 16:56, Tiwei Bie wrote:
> On Thu, May 10, 2018 at 03:34:50PM +0800, Jason Wang wrote:
>> On 2018年05月10日 15:32, Jason Wang wrote:
>>> On 2018年04月25日 13:15, Tiwei Bie wrote:
>>>> + /* We're using some buffers from the free list. */
>>>> + vq->vq.num_free -= descs_used;
>>>> +
>>>> + /* Update free pointer */
>>>> + if (indirect) {
>>>> + n = head + 1;
>>>> + if (n >= vq->vring_packed.num) {
>>>> + n = 0;
>>>> + vq->wrap_counter ^= 1;
>>>> + }
>>>> + vq->next_avail_idx = n;
>>>> + } else
>>>> + vq->next_avail_idx = i;
>>> During testing zerocopy (out of order completion), I found driver may
>>> submit two identical buffer id to vhost. So the above code may not work
>>> well.
>>>
>>> Consider the case that driver adds 3 buffer and virtqueue size is 8.
>>>
>>> a) id = 0,count = 2,next_avail = 2
>>>
>>> b) id = 2,count = 4,next_avail = 2
>> next_avail should be 6 here.
>>
>>> c) id = 4,count = 2,next_avail = 0
>>>
>> id should be 6 here.
>>
>> Thanks
>>
>>> if packet b is done before packet a, driver may think buffer id 0 is
>>> available and try to use it if even if the real buffer 0 was not done.
>>>
>>> Thanks
> Nice catch! Thanks a lot!
> I'll implement an ID allocator.
>
> Best regards,
> Tiwei Bie
Sounds good.
Another similar issue is detac_buf_packed(). It did:
for (j = 0; j < vq->desc_state[head].num; j++) {
desc = &vq->vring_packed.desc[i];
vring_unmap_one_packed(vq, desc);
i++;
if (i >= vq->vring_packed.num)
i = 0;
}
This probably won't work for out of order too and according to the spec:
"""
Driver needs to keep track of the size of the list corresponding to each
buffer ID, to be able to skip to where the next used descriptor is
written by the device.
"""
Looks like we should not depend on the descriptor ring.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-05-10 8:56 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, wexu, virtualization, linux-kernel, mst
In-Reply-To: <5885acac-e9e3-3abf-b6a2-7347f4d55be2@redhat.com>
On Thu, May 10, 2018 at 03:34:50PM +0800, Jason Wang wrote:
> On 2018年05月10日 15:32, Jason Wang wrote:
> > On 2018年04月25日 13:15, Tiwei Bie wrote:
> > > + /* We're using some buffers from the free list. */
> > > + vq->vq.num_free -= descs_used;
> > > +
> > > + /* Update free pointer */
> > > + if (indirect) {
> > > + n = head + 1;
> > > + if (n >= vq->vring_packed.num) {
> > > + n = 0;
> > > + vq->wrap_counter ^= 1;
> > > + }
> > > + vq->next_avail_idx = n;
> > > + } else
> > > + vq->next_avail_idx = i;
> >
> > During testing zerocopy (out of order completion), I found driver may
> > submit two identical buffer id to vhost. So the above code may not work
> > well.
> >
> > Consider the case that driver adds 3 buffer and virtqueue size is 8.
> >
> > a) id = 0,count = 2,next_avail = 2
> >
> > b) id = 2,count = 4,next_avail = 2
>
> next_avail should be 6 here.
>
> >
> > c) id = 4,count = 2,next_avail = 0
> >
>
> id should be 6 here.
>
> Thanks
>
> > if packet b is done before packet a, driver may think buffer id 0 is
> > available and try to use it if even if the real buffer 0 was not done.
> >
> > Thanks
Nice catch! Thanks a lot!
I'll implement an ID allocator.
Best regards,
Tiwei Bie
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Jason Wang @ 2018-05-10 7:34 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, netdev; +Cc: wexu
In-Reply-To: <927f4478-5a81-31d4-ac69-f9ec26248591@redhat.com>
On 2018年05月10日 15:32, Jason Wang wrote:
>
>
> On 2018年04月25日 13:15, Tiwei Bie wrote:
>> + /* We're using some buffers from the free list. */
>> + vq->vq.num_free -= descs_used;
>> +
>> + /* Update free pointer */
>> + if (indirect) {
>> + n = head + 1;
>> + if (n >= vq->vring_packed.num) {
>> + n = 0;
>> + vq->wrap_counter ^= 1;
>> + }
>> + vq->next_avail_idx = n;
>> + } else
>> + vq->next_avail_idx = i;
>
> During testing zerocopy (out of order completion), I found driver may
> submit two identical buffer id to vhost. So the above code may not
> work well.
>
> Consider the case that driver adds 3 buffer and virtqueue size is 8.
>
> a) id = 0,count = 2,next_avail = 2
>
> b) id = 2,count = 4,next_avail = 2
next_avail should be 6 here.
>
> c) id = 4,count = 2,next_avail = 0
>
id should be 6 here.
Thanks
> if packet b is done before packet a, driver may think buffer id 0 is
> available and try to use it if even if the real buffer 0 was not done.
>
> Thanks
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Jason Wang @ 2018-05-10 7:32 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, netdev; +Cc: wexu
In-Reply-To: <20180425051550.24342-4-tiwei.bie@intel.com>
On 2018年04月25日 13:15, Tiwei Bie wrote:
> + /* We're using some buffers from the free list. */
> + vq->vq.num_free -= descs_used;
> +
> + /* Update free pointer */
> + if (indirect) {
> + n = head + 1;
> + if (n >= vq->vring_packed.num) {
> + n = 0;
> + vq->wrap_counter ^= 1;
> + }
> + vq->next_avail_idx = n;
> + } else
> + vq->next_avail_idx = i;
During testing zerocopy (out of order completion), I found driver may
submit two identical buffer id to vhost. So the above code may not work
well.
Consider the case that driver adds 3 buffer and virtqueue size is 8.
a) id = 0,count = 2,next_avail = 2
b) id = 2,count = 4,next_avail = 2
c) id = 4,count = 2,next_avail = 0
if packet b is done before packet a, driver may think buffer id 0 is
available and try to use it if even if the real buffer 0 was not done.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Jason Wang @ 2018-05-09 3:43 UTC (permalink / raw)
To: Tiwei Bie; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <20180508094406.qjlaism3hqy4hvjd@debian>
On 2018年05月08日 17:44, Tiwei Bie wrote:
> On Tue, May 08, 2018 at 05:34:40PM +0800, Jason Wang wrote:
>> On 2018年05月08日 17:16, Tiwei Bie wrote:
>>> On Tue, May 08, 2018 at 03:16:53PM +0800, Jason Wang wrote:
>>>> On 2018年05月08日 14:44, Tiwei Bie wrote:
>>>>> On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
>>>>>> On 2018年05月08日 11:05, Jason Wang wrote:
>>>>>>>> Because in virtqueue_enable_cb_delayed(), we may set an
>>>>>>>> event_off which is bigger than new and both of them have
>>>>>>>> wrapped. And in this case, although new is smaller than
>>>>>>>> event_off (i.e. the third param -- old), new shouldn't
>>>>>>>> add vq->num, and actually we are expecting a very big
>>>>>>>> idx diff.
>>>>>>> Yes, so to calculate distance correctly between event and new, we just
>>>>>>> need to compare the warp counter and return false if it doesn't match
>>>>>>> without the need to try to add vq.num here.
>>>>>>>
>>>>>>> Thanks
>>>>>> Sorry, looks like the following should work, we need add vq.num if
>>>>>> used_wrap_counter does not match:
>>>>>>
>>>>>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>>>>>> __u16 off_wrap, __u16 new,
>>>>>> __u16 old)
>>>>>> {
>>>>>> bool wrap = off_wrap >> 15;
>>>>>> int off = off_wrap & ~(1 << 15);
>>>>>> __u16 d1, d2;
>>>>>>
>>>>>> if (wrap != vq->used_wrap_counter)
>>>>>> d1 = new + vq->num - off - 1;
>>>>> Just to draw your attention (maybe you have already
>>>>> noticed this).
>>>> I miss this, thanks!
>>>>
>>>>> In this case (i.e. wrap != vq->used_wrap_counter),
>>>>> it's also possible that (off < new) is true. Because,
>>>>>
>>>>> when virtqueue_enable_cb_delayed_packed() is used,
>>>>> `off` is calculated in driver in a way like this:
>>>>>
>>>>> off = vq->last_used_idx + bufs;
>>>>> if (off >= vq->vring_packed.num) {
>>>>> off -= vq->vring_packed.num;
>>>>> wrap_counter ^= 1;
>>>>> }
>>>>>
>>>>> And when `new` (in vhost) is close to vq->num. The
>>>>> vq->last_used_idx + bufs (in driver) can be bigger
>>>>> than vq->vring_packed.num, and:
>>>>>
>>>>> 1. `off` will wrap;
>>>>> 2. wrap counters won't match;
>>>>> 3. off < new;
>>>>>
>>>>> And d1 (i.e. new + vq->num - off - 1) will be a value
>>>>> bigger than vq->num. I'm okay with this, although it's
>>>>> a bit weird.
>>>> So I'm considering something more compact by reusing vring_need_event() by
>>>> pretending a larger queue size and adding vq->num back when necessary:
>>>>
>>>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>>>> __u16 off_wrap, __u16 new,
>>>> __u16 old)
>>>> {
>>>> bool wrap = vq->used_wrap_counter;
>>> If the wrap counter is obtained from the vq,
>>> I think `new` should also be obtained from
>>> the vq. Or the wrap counter should be carried
>>> in `new`.
>>>
>>>> int off = off_wrap & ~(1 << 15);
>>>> __u16 d1, d2;
>>>>
>>>> if (new < old) {
>>>> new += vq->num;
>>>> wrap ^= 1;
>>>> }
>>>>
>>>> if (wrap != off_wrap >> 15)
>>>> off += vq->num;
>>> When `new` and `old` wraps, and `off` doesn't wrap,
>>> wrap != (off_wrap >> 15) will be true. In this case,
>>> `off` is bigger than `new`, and what we should do
>>> is `off -= vq->num` instead of `off += vq->num`.
>> If I understand this correctly, if we track old correctly, it won't happen
>> if guest driver behave correctly. That means it should only happen for a
>> buggy driver (e.g trying to move off_wrap back).
> If vhost is faster than virtio driver, I guess above
> case may happen. The `old` and `new` will be updated
> each time we want to notify the driver. If the driver
> is slower, `old` and `new` in vhost may wrap before
> the `off` which is set by driver wraps.
>
> Best regards,
> Tiwei Bie
>
Oh, right.
But the code still work (in this case new - event_idx - 1 will
underflow). (And I admit it still looks ugly).
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-05-08 9:44 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <122277c6-d103-e1f6-d695-4d64e6934a51@redhat.com>
On Tue, May 08, 2018 at 05:34:40PM +0800, Jason Wang wrote:
> On 2018年05月08日 17:16, Tiwei Bie wrote:
> > On Tue, May 08, 2018 at 03:16:53PM +0800, Jason Wang wrote:
> > > On 2018年05月08日 14:44, Tiwei Bie wrote:
> > > > On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
> > > > > On 2018年05月08日 11:05, Jason Wang wrote:
> > > > > > > Because in virtqueue_enable_cb_delayed(), we may set an
> > > > > > > event_off which is bigger than new and both of them have
> > > > > > > wrapped. And in this case, although new is smaller than
> > > > > > > event_off (i.e. the third param -- old), new shouldn't
> > > > > > > add vq->num, and actually we are expecting a very big
> > > > > > > idx diff.
> > > > > > Yes, so to calculate distance correctly between event and new, we just
> > > > > > need to compare the warp counter and return false if it doesn't match
> > > > > > without the need to try to add vq.num here.
> > > > > >
> > > > > > Thanks
> > > > > Sorry, looks like the following should work, we need add vq.num if
> > > > > used_wrap_counter does not match:
> > > > >
> > > > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
> > > > > __u16 off_wrap, __u16 new,
> > > > > __u16 old)
> > > > > {
> > > > > bool wrap = off_wrap >> 15;
> > > > > int off = off_wrap & ~(1 << 15);
> > > > > __u16 d1, d2;
> > > > >
> > > > > if (wrap != vq->used_wrap_counter)
> > > > > d1 = new + vq->num - off - 1;
> > > > Just to draw your attention (maybe you have already
> > > > noticed this).
> > > I miss this, thanks!
> > >
> > > > In this case (i.e. wrap != vq->used_wrap_counter),
> > > > it's also possible that (off < new) is true. Because,
> > > >
> > > > when virtqueue_enable_cb_delayed_packed() is used,
> > > > `off` is calculated in driver in a way like this:
> > > >
> > > > off = vq->last_used_idx + bufs;
> > > > if (off >= vq->vring_packed.num) {
> > > > off -= vq->vring_packed.num;
> > > > wrap_counter ^= 1;
> > > > }
> > > >
> > > > And when `new` (in vhost) is close to vq->num. The
> > > > vq->last_used_idx + bufs (in driver) can be bigger
> > > > than vq->vring_packed.num, and:
> > > >
> > > > 1. `off` will wrap;
> > > > 2. wrap counters won't match;
> > > > 3. off < new;
> > > >
> > > > And d1 (i.e. new + vq->num - off - 1) will be a value
> > > > bigger than vq->num. I'm okay with this, although it's
> > > > a bit weird.
> > >
> > > So I'm considering something more compact by reusing vring_need_event() by
> > > pretending a larger queue size and adding vq->num back when necessary:
> > >
> > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
> > > __u16 off_wrap, __u16 new,
> > > __u16 old)
> > > {
> > > bool wrap = vq->used_wrap_counter;
> > If the wrap counter is obtained from the vq,
> > I think `new` should also be obtained from
> > the vq. Or the wrap counter should be carried
> > in `new`.
> >
> > > int off = off_wrap & ~(1 << 15);
> > > __u16 d1, d2;
> > >
> > > if (new < old) {
> > > new += vq->num;
> > > wrap ^= 1;
> > > }
> > >
> > > if (wrap != off_wrap >> 15)
> > > off += vq->num;
> > When `new` and `old` wraps, and `off` doesn't wrap,
> > wrap != (off_wrap >> 15) will be true. In this case,
> > `off` is bigger than `new`, and what we should do
> > is `off -= vq->num` instead of `off += vq->num`.
>
> If I understand this correctly, if we track old correctly, it won't happen
> if guest driver behave correctly. That means it should only happen for a
> buggy driver (e.g trying to move off_wrap back).
If vhost is faster than virtio driver, I guess above
case may happen. The `old` and `new` will be updated
each time we want to notify the driver. If the driver
is slower, `old` and `new` in vhost may wrap before
the `off` which is set by driver wraps.
Best regards,
Tiwei Bie
>
> Thanks
>
> >
> > Best regards,
> > Tiwei Bie
> >
> > > return vring_need_event(off, new, old);
> > > }
> > >
> > >
> > > > Best regards,
> > > > Tiwei Bie
> > > >
> > > > > else
> > > > > d1 = new - off - 1;
> > > > >
> > > > > if (new > old)
> > > > > d2 = new - old;
> > > > > else
> > > > > d2 = new + vq->num - old;
> > > > >
> > > > > return d1 < d2;
> > > > > }
> > > > >
> > > > > Thanks
> > > > >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Jason Wang @ 2018-05-08 9:34 UTC (permalink / raw)
To: Tiwei Bie; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <20180508091628.d7jzpopqopq4abhy@debian>
On 2018年05月08日 17:16, Tiwei Bie wrote:
> On Tue, May 08, 2018 at 03:16:53PM +0800, Jason Wang wrote:
>> On 2018年05月08日 14:44, Tiwei Bie wrote:
>>> On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
>>>> On 2018年05月08日 11:05, Jason Wang wrote:
>>>>>> Because in virtqueue_enable_cb_delayed(), we may set an
>>>>>> event_off which is bigger than new and both of them have
>>>>>> wrapped. And in this case, although new is smaller than
>>>>>> event_off (i.e. the third param -- old), new shouldn't
>>>>>> add vq->num, and actually we are expecting a very big
>>>>>> idx diff.
>>>>> Yes, so to calculate distance correctly between event and new, we just
>>>>> need to compare the warp counter and return false if it doesn't match
>>>>> without the need to try to add vq.num here.
>>>>>
>>>>> Thanks
>>>> Sorry, looks like the following should work, we need add vq.num if
>>>> used_wrap_counter does not match:
>>>>
>>>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>>>> __u16 off_wrap, __u16 new,
>>>> __u16 old)
>>>> {
>>>> bool wrap = off_wrap >> 15;
>>>> int off = off_wrap & ~(1 << 15);
>>>> __u16 d1, d2;
>>>>
>>>> if (wrap != vq->used_wrap_counter)
>>>> d1 = new + vq->num - off - 1;
>>> Just to draw your attention (maybe you have already
>>> noticed this).
>> I miss this, thanks!
>>
>>> In this case (i.e. wrap != vq->used_wrap_counter),
>>> it's also possible that (off < new) is true. Because,
>>>
>>> when virtqueue_enable_cb_delayed_packed() is used,
>>> `off` is calculated in driver in a way like this:
>>>
>>> off = vq->last_used_idx + bufs;
>>> if (off >= vq->vring_packed.num) {
>>> off -= vq->vring_packed.num;
>>> wrap_counter ^= 1;
>>> }
>>>
>>> And when `new` (in vhost) is close to vq->num. The
>>> vq->last_used_idx + bufs (in driver) can be bigger
>>> than vq->vring_packed.num, and:
>>>
>>> 1. `off` will wrap;
>>> 2. wrap counters won't match;
>>> 3. off < new;
>>>
>>> And d1 (i.e. new + vq->num - off - 1) will be a value
>>> bigger than vq->num. I'm okay with this, although it's
>>> a bit weird.
>>
>> So I'm considering something more compact by reusing vring_need_event() by
>> pretending a larger queue size and adding vq->num back when necessary:
>>
>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>> __u16 off_wrap, __u16 new,
>> __u16 old)
>> {
>> bool wrap = vq->used_wrap_counter;
> If the wrap counter is obtained from the vq,
> I think `new` should also be obtained from
> the vq. Or the wrap counter should be carried
> in `new`.
>
>> int off = off_wrap & ~(1 << 15);
>> __u16 d1, d2;
>>
>> if (new < old) {
>> new += vq->num;
>> wrap ^= 1;
>> }
>>
>> if (wrap != off_wrap >> 15)
>> off += vq->num;
> When `new` and `old` wraps, and `off` doesn't wrap,
> wrap != (off_wrap >> 15) will be true. In this case,
> `off` is bigger than `new`, and what we should do
> is `off -= vq->num` instead of `off += vq->num`.
If I understand this correctly, if we track old correctly, it won't
happen if guest driver behave correctly. That means it should only
happen for a buggy driver (e.g trying to move off_wrap back).
Thanks
>
> Best regards,
> Tiwei Bie
>
>> return vring_need_event(off, new, old);
>> }
>>
>>
>>> Best regards,
>>> Tiwei Bie
>>>
>>>> else
>>>> d1 = new - off - 1;
>>>>
>>>> if (new > old)
>>>> d2 = new - old;
>>>> else
>>>> d2 = new + vq->num - old;
>>>>
>>>> return d1 < d2;
>>>> }
>>>>
>>>> Thanks
>>>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-05-08 9:16 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <34f2c690-7cb2-f9ea-2ce9-40f4ccb594c9@redhat.com>
On Tue, May 08, 2018 at 03:16:53PM +0800, Jason Wang wrote:
> On 2018年05月08日 14:44, Tiwei Bie wrote:
> > On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
> > > On 2018年05月08日 11:05, Jason Wang wrote:
> > > > > Because in virtqueue_enable_cb_delayed(), we may set an
> > > > > event_off which is bigger than new and both of them have
> > > > > wrapped. And in this case, although new is smaller than
> > > > > event_off (i.e. the third param -- old), new shouldn't
> > > > > add vq->num, and actually we are expecting a very big
> > > > > idx diff.
> > > > Yes, so to calculate distance correctly between event and new, we just
> > > > need to compare the warp counter and return false if it doesn't match
> > > > without the need to try to add vq.num here.
> > > >
> > > > Thanks
> > > Sorry, looks like the following should work, we need add vq.num if
> > > used_wrap_counter does not match:
> > >
> > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
> > > __u16 off_wrap, __u16 new,
> > > __u16 old)
> > > {
> > > bool wrap = off_wrap >> 15;
> > > int off = off_wrap & ~(1 << 15);
> > > __u16 d1, d2;
> > >
> > > if (wrap != vq->used_wrap_counter)
> > > d1 = new + vq->num - off - 1;
> > Just to draw your attention (maybe you have already
> > noticed this).
>
> I miss this, thanks!
>
> >
> > In this case (i.e. wrap != vq->used_wrap_counter),
> > it's also possible that (off < new) is true. Because,
> >
> > when virtqueue_enable_cb_delayed_packed() is used,
> > `off` is calculated in driver in a way like this:
> >
> > off = vq->last_used_idx + bufs;
> > if (off >= vq->vring_packed.num) {
> > off -= vq->vring_packed.num;
> > wrap_counter ^= 1;
> > }
> >
> > And when `new` (in vhost) is close to vq->num. The
> > vq->last_used_idx + bufs (in driver) can be bigger
> > than vq->vring_packed.num, and:
> >
> > 1. `off` will wrap;
> > 2. wrap counters won't match;
> > 3. off < new;
> >
> > And d1 (i.e. new + vq->num - off - 1) will be a value
> > bigger than vq->num. I'm okay with this, although it's
> > a bit weird.
>
>
> So I'm considering something more compact by reusing vring_need_event() by
> pretending a larger queue size and adding vq->num back when necessary:
>
> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
> __u16 off_wrap, __u16 new,
> __u16 old)
> {
> bool wrap = vq->used_wrap_counter;
If the wrap counter is obtained from the vq,
I think `new` should also be obtained from
the vq. Or the wrap counter should be carried
in `new`.
> int off = off_wrap & ~(1 << 15);
> __u16 d1, d2;
>
> if (new < old) {
> new += vq->num;
> wrap ^= 1;
> }
>
> if (wrap != off_wrap >> 15)
> off += vq->num;
When `new` and `old` wraps, and `off` doesn't wrap,
wrap != (off_wrap >> 15) will be true. In this case,
`off` is bigger than `new`, and what we should do
is `off -= vq->num` instead of `off += vq->num`.
Best regards,
Tiwei Bie
>
> return vring_need_event(off, new, old);
> }
>
>
> >
> > Best regards,
> > Tiwei Bie
> >
> > > else
> > > d1 = new - off - 1;
> > >
> > > if (new > old)
> > > d2 = new - old;
> > > else
> > > d2 = new + vq->num - old;
> > >
> > > return d1 < d2;
> > > }
> > >
> > > Thanks
> > >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: kbuild test robot @ 2018-05-08 9:02 UTC (permalink / raw)
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, kbuild-all, sridhar.samudrala,
aaron.f.brown, davem
In-Reply-To: <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
Hi Sridhar,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180508-123531
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/core/net_failover.c:868:16: sparse: Using plain integer as NULL pointer
net/core/net_failover.c:115:12: sparse: context imbalance in 'net_failover_select_queue' - wrong count at exit
vim +868 net/core/net_failover.c
828
829 /**
830 * net_failover_register - Register a failover instance
831 *
832 * @dev: failover or standby netdev
833 * @ops: failover ops
834 *
835 * Paravirtual drivers supporting 3-netdev model call this routine indirectly
836 * via net_failover_create(). It passes failover netdev and ops will be NULL
837 * as the slave events are handled internally.
838 * Paravirtual drivers supporting 2-netdev model call this routine by passing
839 * standby netdev and ops that are called to handle slave register/unregister/
840 * link change events.
841 *
842 * Return: pointer to failover instance
843 */
844 struct net_failover *net_failover_register(struct net_device *dev,
845 struct net_failover_ops *ops)
846 {
847 struct net_failover *failover;
848
849 failover = kzalloc(sizeof(*failover), GFP_KERNEL);
850 if (!failover)
851 return ERR_PTR(-ENOMEM);
852
853 rcu_assign_pointer(failover->ops, ops);
854 dev_hold(dev);
855 dev->priv_flags |= IFF_FAILOVER;
856 rcu_assign_pointer(failover->failover_dev, dev);
857
858 spin_lock(&net_failover_lock);
859 list_add_tail(&failover->list, &net_failover_list);
860 spin_unlock(&net_failover_lock);
861
862 netdev_info(dev, "failover master:%s registered\n", dev->name);
863
864 net_failover_existing_slave_register(dev);
865
866 return failover;
867
> 868 return 0;
869 }
870 EXPORT_SYMBOL_GPL(net_failover_register);
871
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Jason Wang @ 2018-05-08 7:16 UTC (permalink / raw)
To: Tiwei Bie; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <20180508064409.kcn6amhsxu7nkuuc@debian>
On 2018年05月08日 14:44, Tiwei Bie wrote:
> On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
>> On 2018年05月08日 11:05, Jason Wang wrote:
>>>> Because in virtqueue_enable_cb_delayed(), we may set an
>>>> event_off which is bigger than new and both of them have
>>>> wrapped. And in this case, although new is smaller than
>>>> event_off (i.e. the third param -- old), new shouldn't
>>>> add vq->num, and actually we are expecting a very big
>>>> idx diff.
>>> Yes, so to calculate distance correctly between event and new, we just
>>> need to compare the warp counter and return false if it doesn't match
>>> without the need to try to add vq.num here.
>>>
>>> Thanks
>> Sorry, looks like the following should work, we need add vq.num if
>> used_wrap_counter does not match:
>>
>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>> __u16 off_wrap, __u16 new,
>> __u16 old)
>> {
>> bool wrap = off_wrap >> 15;
>> int off = off_wrap & ~(1 << 15);
>> __u16 d1, d2;
>>
>> if (wrap != vq->used_wrap_counter)
>> d1 = new + vq->num - off - 1;
> Just to draw your attention (maybe you have already
> noticed this).
I miss this, thanks!
>
> In this case (i.e. wrap != vq->used_wrap_counter),
> it's also possible that (off < new) is true. Because,
>
> when virtqueue_enable_cb_delayed_packed() is used,
> `off` is calculated in driver in a way like this:
>
> off = vq->last_used_idx + bufs;
> if (off >= vq->vring_packed.num) {
> off -= vq->vring_packed.num;
> wrap_counter ^= 1;
> }
>
> And when `new` (in vhost) is close to vq->num. The
> vq->last_used_idx + bufs (in driver) can be bigger
> than vq->vring_packed.num, and:
>
> 1. `off` will wrap;
> 2. wrap counters won't match;
> 3. off < new;
>
> And d1 (i.e. new + vq->num - off - 1) will be a value
> bigger than vq->num. I'm okay with this, although it's
> a bit weird.
So I'm considering something more compact by reusing vring_need_event()
by pretending a larger queue size and adding vq->num back when necessary:
static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
__u16 off_wrap, __u16 new,
__u16 old)
{
bool wrap = vq->used_wrap_counter;
int off = off_wrap & ~(1 << 15);
__u16 d1, d2;
if (new < old) {
new += vq->num;
wrap ^= 1;
}
if (wrap != off_wrap >> 15)
off += vq->num;
return vring_need_event(off, new, old);
}
>
> Best regards,
> Tiwei Bie
>
>> else
>> d1 = new - off - 1;
>>
>> if (new > old)
>> d2 = new - old;
>> else
>> d2 = new + vq->num - old;
>>
>> return d1 < d2;
>> }
>>
>> Thanks
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-05-08 6:44 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <12ede490-f674-2b89-d639-266b5fe15466@redhat.com>
On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
> On 2018年05月08日 11:05, Jason Wang wrote:
> > >
> > > Because in virtqueue_enable_cb_delayed(), we may set an
> > > event_off which is bigger than new and both of them have
> > > wrapped. And in this case, although new is smaller than
> > > event_off (i.e. the third param -- old), new shouldn't
> > > add vq->num, and actually we are expecting a very big
> > > idx diff.
> >
> > Yes, so to calculate distance correctly between event and new, we just
> > need to compare the warp counter and return false if it doesn't match
> > without the need to try to add vq.num here.
> >
> > Thanks
>
> Sorry, looks like the following should work, we need add vq.num if
> used_wrap_counter does not match:
>
> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
> __u16 off_wrap, __u16 new,
> __u16 old)
> {
> bool wrap = off_wrap >> 15;
> int off = off_wrap & ~(1 << 15);
> __u16 d1, d2;
>
> if (wrap != vq->used_wrap_counter)
> d1 = new + vq->num - off - 1;
Just to draw your attention (maybe you have already
noticed this).
In this case (i.e. wrap != vq->used_wrap_counter),
it's also possible that (off < new) is true. Because,
when virtqueue_enable_cb_delayed_packed() is used,
`off` is calculated in driver in a way like this:
off = vq->last_used_idx + bufs;
if (off >= vq->vring_packed.num) {
off -= vq->vring_packed.num;
wrap_counter ^= 1;
}
And when `new` (in vhost) is close to vq->num. The
vq->last_used_idx + bufs (in driver) can be bigger
than vq->vring_packed.num, and:
1. `off` will wrap;
2. wrap counters won't match;
3. off < new;
And d1 (i.e. new + vq->num - off - 1) will be a value
bigger than vq->num. I'm okay with this, although it's
a bit weird.
Best regards,
Tiwei Bie
> else
> d1 = new - off - 1;
>
> if (new > old)
> d2 = new - old;
> else
> d2 = new + vq->num - old;
>
> return d1 < d2;
> }
>
> Thanks
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Jason Wang @ 2018-05-08 5:40 UTC (permalink / raw)
To: Tiwei Bie; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <fbe06940-1618-5d34-7321-f4888c5dd1ee@redhat.com>
On 2018年05月08日 11:05, Jason Wang wrote:
>>
>> Because in virtqueue_enable_cb_delayed(), we may set an
>> event_off which is bigger than new and both of them have
>> wrapped. And in this case, although new is smaller than
>> event_off (i.e. the third param -- old), new shouldn't
>> add vq->num, and actually we are expecting a very big
>> idx diff.
>
> Yes, so to calculate distance correctly between event and new, we just
> need to compare the warp counter and return false if it doesn't match
> without the need to try to add vq.num here.
>
> Thanks
Sorry, looks like the following should work, we need add vq.num if
used_wrap_counter does not match:
static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
__u16 off_wrap, __u16 new,
__u16 old)
{
bool wrap = off_wrap >> 15;
int off = off_wrap & ~(1 << 15);
__u16 d1, d2;
if (wrap != vq->used_wrap_counter)
d1 = new + vq->num - off - 1;
else
d1 = new - off - 1;
if (new > old)
d2 = new - old;
else
d2 = new + vq->num - old;
return d1 < d2;
}
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Jason Wang @ 2018-05-08 3:05 UTC (permalink / raw)
To: Tiwei Bie; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <20180503135430.lbtvn4p4lyu3ksqo@debian>
On 2018年05月03日 21:54, Tiwei Bie wrote:
> On Thu, May 03, 2018 at 03:25:29PM +0800, Jason Wang wrote:
>> On 2018年05月03日 10:09, Tiwei Bie wrote:
>>>>>> So how about we use the straightforward way then?
>>>>> You mean we do new += vq->vring_packed.num instead
>>>>> of event_idx -= vq->vring_packed.num before calling
>>>>> vring_need_event()?
>>>>>
>>>>> The problem is that, the second param (new_idx) of
>>>>> vring_need_event() will be used for:
>>>>>
>>>>> (__u16)(new_idx - event_idx - 1)
>>>>> (__u16)(new_idx - old)
>>>>>
>>>>> So if we change new, we will need to change old too.
>>>> I think that since we have a branch there anyway,
>>>> we are better off just special-casing if (wrap_counter != vq->wrap_counter).
>>>> Treat is differenty and avoid casts.
>>>>
>>>>> And that would be an ugly hack..
>>>>>
>>>>> Best regards,
>>>>> Tiwei Bie
>>>> I consider casts and huge numbers with two's complement
>>>> games even uglier.
>>> The dependency on two's complement game is introduced
>>> since the split ring.
>>>
>>> In packed ring, old is calculated via:
>>>
>>> old = vq->next_avail_idx - vq->num_added;
>>>
>>> In split ring, old is calculated via:
>>>
>>> old = vq->avail_idx_shadow - vq->num_added;
>>>
>>> In both cases, when vq->num_added is bigger, old will
>>> be a big number.
>>>
>>> Best regards,
>>> Tiwei Bie
>>>
>> How about just do something like vhost:
>>
>> static u16 vhost_idx_diff(struct vhost_virtqueue *vq, u16 old, u16 new)
>> {
>> if (new > old)
>> return new - old;
>> return (new + vq->num - old);
>> }
>>
>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>> __u16 event_off, __u16 new,
>> __u16 old)
>> {
>> return (__u16)(vhost_idx_diff(vq, new, event_off) - 1) <
>> (__u16)vhost_idx_diff(vq, new, old);
>> }
>>
>> ?
> It seems that there is a typo in above code. The second
> param of vhost_idx_diff() is `old`, but when calling this
> function in vhost_vring_packed_need_event(), `new` is
> passed as the second param.
Right.
>
> If we assume the second param of vhost_idx_diff() is new
> and the third one is old, i.e.:
>
> static u16 vhost_idx_diff(struct vhost_virtqueue *vq, u16 new, u16 old)
> {
> if (new > old)
> return new - old;
> return (new + vq->num - old);
> }
>
> I think it's still not right.
>
> Because in virtqueue_enable_cb_delayed(), we may set an
> event_off which is bigger than new and both of them have
> wrapped. And in this case, although new is smaller than
> event_off (i.e. the third param -- old), new shouldn't
> add vq->num, and actually we are expecting a very big
> idx diff.
Yes, so to calculate distance correctly between event and new, we just
need to compare the warp counter and return false if it doesn't match
without the need to try to add vq.num here.
Thanks
>
> Best regards,
> Tiwei Bie
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Samudrala, Sridhar @ 2018-05-08 0:24 UTC (permalink / raw)
To: Stephen Hemminger
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, aaron.f.brown, davem
In-Reply-To: <20180507165306.541bd1f2@xeon-e3>
On 5/7/2018 4:53 PM, Stephen Hemminger wrote:
> On Mon, 7 May 2018 15:10:44 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
>> +static struct net_device *net_failover_get_bymac(u8 *mac,
>> + struct net_failover_ops **ops)
>> +{
>> + struct net_device *failover_dev;
>> + struct net_failover *failover;
>> +
>> + spin_lock(&net_failover_lock);
>> + list_for_each_entry(failover, &net_failover_list, list) {
>> + failover_dev = rtnl_dereference(failover->failover_dev);
>> + if (ether_addr_equal(failover_dev->perm_addr, mac)) {
>> + *ops = rtnl_dereference(failover->ops);
>> + spin_unlock(&net_failover_lock);
>> + return failover_dev;
>> + }
>> + }
>> + spin_unlock(&net_failover_lock);
>> + return NULL;
>> +}
> This is broken if non-ethernet devices such as Infiniband are present.
There is check to make sure that a slave and failover devices are of the same type in
net_failover_slave_register()
failover_dev = net_failover_get_bymac(slave_dev->perm_addr, &nfo_ops);
if (!failover_dev)
goto done;
if (failover_dev->type != slave_dev->type)
goto done;
Do you think this is not good enough? I had an explicit check for ARPHRD_ETHER in
earlier patchsets, but removed it based on Jiri's comment.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Samudrala, Sridhar @ 2018-05-08 0:11 UTC (permalink / raw)
To: Stephen Hemminger
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, aaron.f.brown, davem
In-Reply-To: <20180507165911.45b14c58@xeon-e3>
On 5/7/2018 4:59 PM, Stephen Hemminger wrote:
> On Mon, 7 May 2018 15:10:44 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
>> + if (netif_running(failover_dev)) {
>> + err = dev_open(slave_dev);
>> + if (err && (err != -EBUSY)) {
>> + netdev_err(failover_dev, "Opening slave %s failed err:%d\n",
>> + slave_dev->name, err);
>> + goto err_dev_open;
>> + }
>> + }
>> +
>> + netif_addr_lock_bh(failover_dev);
>> + dev_uc_sync_multiple(slave_dev, failover_dev);
>> + dev_uc_sync_multiple(slave_dev, failover_dev);
>> + netif_addr_unlock_bh(failover_dev);
>> +
> The order of these is backwards, you want to sync addresses before bringing up.
> Also, doing it this way does not allow udev/systemd the chance to rename VF devices.
During my testing, i noticed that dev_open() may fail with EBUSY in certain scenarios,
If so, the opening of the slave is handled after the rename via the NETDEV_CHANGENAME
event handler.
>
> The complexity of this whole failover mechanism does not make life easier,
> more reliable, or safer for netvsc. I though that was the whole reason for having
> common code.
netvsc doesn't go through this code.
if (nfo_ops && nfo_ops->slave_register)
return nfo_ops->slave_register(slave_dev, failover_dev);
So there is no change in event handling for netvsc 2-netdev model.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Stephen Hemminger @ 2018-05-07 23:59 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, aaron.f.brown, davem
In-Reply-To: <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
On Mon, 7 May 2018 15:10:44 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> + if (netif_running(failover_dev)) {
> + err = dev_open(slave_dev);
> + if (err && (err != -EBUSY)) {
> + netdev_err(failover_dev, "Opening slave %s failed err:%d\n",
> + slave_dev->name, err);
> + goto err_dev_open;
> + }
> + }
> +
> + netif_addr_lock_bh(failover_dev);
> + dev_uc_sync_multiple(slave_dev, failover_dev);
> + dev_uc_sync_multiple(slave_dev, failover_dev);
> + netif_addr_unlock_bh(failover_dev);
> +
The order of these is backwards, you want to sync addresses before bringing up.
Also, doing it this way does not allow udev/systemd the chance to rename VF devices.
The complexity of this whole failover mechanism does not make life easier,
more reliable, or safer for netvsc. I though that was the whole reason for having
common code.
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Stephen Hemminger @ 2018-05-07 23:53 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, aaron.f.brown, davem
In-Reply-To: <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
On Mon, 7 May 2018 15:10:44 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> +static struct net_device *net_failover_get_bymac(u8 *mac,
> + struct net_failover_ops **ops)
> +{
> + struct net_device *failover_dev;
> + struct net_failover *failover;
> +
> + spin_lock(&net_failover_lock);
> + list_for_each_entry(failover, &net_failover_list, list) {
> + failover_dev = rtnl_dereference(failover->failover_dev);
> + if (ether_addr_equal(failover_dev->perm_addr, mac)) {
> + *ops = rtnl_dereference(failover->ops);
> + spin_unlock(&net_failover_lock);
> + return failover_dev;
> + }
> + }
> + spin_unlock(&net_failover_lock);
> + return NULL;
> +}
This is broken if non-ethernet devices such as Infiniband are present.
^ permalink raw reply
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Stephen Hemminger @ 2018-05-07 23:46 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, aaron.f.brown, davem
In-Reply-To: <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
On Mon, 7 May 2018 15:10:44 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> This provides a generic interface for paravirtual drivers to listen
> for netdev register/unregister/link change events from pci ethernet
> devices with the same MAC and takeover their datapath. The notifier and
> event handling code is based on the existing netvsc implementation.
>
> It exposes 2 sets of interfaces to the paravirtual drivers.
> 1. For paravirtual drivers like virtio_net that use 3 netdev model, the
> the failover module provides interfaces to create/destroy additional
> master netdev and all the slave events are managed internally.
> net_failover_create()
> net_failover_destroy()
> A failover netdev is created that acts a master device and controls 2
> slave devices. The original virtio_net netdev is registered as 'standby'
> netdev and a passthru/vf device with the same MAC gets registered as
> 'primary' netdev. Both 'standby' and 'failover' netdevs are associated
> with the same 'pci' device. The user accesses the network interface via
> 'failover' netdev. The 'failover' netdev chooses 'primary' netdev as
> default for transmits when it is available with link up and running.
> 2. For existing netvsc driver that uses 2 netdev model, no master netdev
> is created. The paravirtual driver registers each instance of netvsc
> as a 'failover' netdev along with a set of ops to manage the slave
> events. There is no 'standby' netdev in this model. A passthru/vf device
> with the same MAC gets registered as 'primary' netdev.
> net_failover_register()
> net_failover_unregister()
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
You are conflating the net_failover device (3 device model) with
the generic network failover infrastructure into one file. There should be two
seperate files net/core/failover.c and drivers/net/failover.c which splits
the work into two parts (and acts a check for the api).
^ permalink raw reply
* [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
From: Sridhar Samudrala @ 2018-05-07 23:09 UTC (permalink / raw)
To: mst, netdev, virtualization, virtio-dev, jesse.brandeburg,
alexander.h.duyck, kubakici, sridhar.samudrala, jasowang,
loseweigh, jiri, aaron.f.brown, qemu-devel
This feature bit can be used by hypervisor to indicate virtio_net device to
act as a standby for another device with the same MAC address.
I tested this with a small change to the patch to mark the STANDBY feature 'true'
by default as i am using libvirt to start the VMs.
Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt
XML file?
Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
hw/net/virtio-net.c | 2 ++
include/standard-headers/linux/virtio_net.h | 3 +++
2 files changed, 5 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 90502fca7c..38b3140670 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = {
true),
DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
+ DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY,
+ false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h
index e9f255ea3f..01ec09684c 100644
--- a/include/standard-headers/linux/virtio_net.h
+++ b/include/standard-headers/linux/virtio_net.h
@@ -57,6 +57,9 @@
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device
+ * with the same MAC.
+ */
#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
#ifndef VIRTIO_NET_NO_LEGACY
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
From: Randy Dunlap @ 2018-05-07 22:39 UTC (permalink / raw)
To: Sridhar Samudrala, mst, stephen, davem, netdev, virtualization,
virtio-dev, jesse.brandeburg, alexander.h.duyck, kubakici,
jasowang, loseweigh, jiri, aaron.f.brown
In-Reply-To: <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
Hi,
On 05/07/2018 03:10 PM, Sridhar Samudrala wrote:
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> MAINTAINERS | 7 +
> include/linux/netdevice.h | 16 +
> include/net/net_failover.h | 52 +++
> net/Kconfig | 10 +
> net/core/Makefile | 1 +
> net/core/net_failover.c | 1044 ++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 1130 insertions(+)
> create mode 100644 include/net/net_failover.h
> create mode 100644 net/core/net_failover.c
> diff --git a/net/Kconfig b/net/Kconfig
> index b62089fb1332..0540856676de 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -429,6 +429,16 @@ config MAY_USE_DEVLINK
> config PAGE_POOL
> bool
>
> +config NET_FAILOVER
> + tristate "Failover interface"
> + default m
Need some justification for default m (as opposed to n).
> + help
> + This provides a generic interface for paravirtual drivers to listen
> + for netdev register/unregister/link change events from pci ethernet
PCI
> + devices with the same MAC and takeover their datapath. This also
> + enables live migration of a VM with direct attached VF by failing
> + over to the paravirtual datapath when the VF is unplugged.
> +
> endif # if NET
>
> # Used by archs to tell that they support BPF JIT compiler plus which flavour.
> diff --git a/net/core/net_failover.c b/net/core/net_failover.c
> new file mode 100644
> index 000000000000..8d60e74e3034
> --- /dev/null
> +++ b/net/core/net_failover.c
> @@ -0,0 +1,1044 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, Intel Corporation. */
> +
> +/* A common module to handle registrations and notifications for paravirtual
> + * drivers to enable accelerated datapath and support VF live migration.
> + *
> + * The notifier and event handling code is based on netvsc driver and failover
> + * netdev management routines are based on bond/team driver.
> + *
> + */
> +/**
> + * net_failover_create - Create and register a failover instance
> + *
> + * @dev: standby netdev
* @standby_dev: standby netdev
> + *
> + * Creates a failover netdev and registers a failover instance for a standby
> + * netdev. Used by paravirtual drivers that use 3-netdev model.
> + * The failover netdev acts as a master device and controls 2 slave devices -
> + * the original standby netdev and a VF netdev with the same MAC gets
> + * registered as primary netdev.
> + *
> + * Return: pointer to failover instance
> + */
> +struct net_failover *net_failover_create(struct net_device *standby_dev)
> +{
> + struct device *dev = standby_dev->dev.parent;
> + struct net_device *failover_dev;
> + struct net_failover *failover;
> + int err;
> +
> + /* Alloc at least 2 queues, for now we are going with 16 assuming
> + * that VF devices being enslaved won't have too many queues.
> + */
> + failover_dev = alloc_etherdev_mq(sizeof(struct net_failover_info), 16);
> + if (!failover_dev) {
> + dev_err(dev, "Unable to allocate failover_netdev!\n");
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + dev_net_set(failover_dev, dev_net(standby_dev));
> + SET_NETDEV_DEV(failover_dev, dev);
> +
> + failover_dev->netdev_ops = &failover_dev_ops;
> + failover_dev->ethtool_ops = &failover_ethtool_ops;
> +
> + /* Initialize the device options */
> + failover_dev->priv_flags |= IFF_UNICAST_FLT | IFF_NO_QUEUE;
> + failover_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE |
> + IFF_TX_SKB_SHARING);
> +
> + /* don't acquire failover netdev's netif_tx_lock when transmitting */
> + failover_dev->features |= NETIF_F_LLTX;
> +
> + /* Don't allow failover devices to change network namespaces. */
> + failover_dev->features |= NETIF_F_NETNS_LOCAL;
> +
> + failover_dev->hw_features = FAILOVER_VLAN_FEATURES |
> + NETIF_F_HW_VLAN_CTAG_TX |
> + NETIF_F_HW_VLAN_CTAG_RX |
> + NETIF_F_HW_VLAN_CTAG_FILTER;
> +
> + failover_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
> + failover_dev->features |= failover_dev->hw_features;
> +
> + memcpy(failover_dev->dev_addr, standby_dev->dev_addr,
> + failover_dev->addr_len);
> +
> + failover_dev->min_mtu = standby_dev->min_mtu;
> + failover_dev->max_mtu = standby_dev->max_mtu;
> +
> + err = register_netdev(failover_dev);
> + if (err) {
> + dev_err(dev, "Unable to register failover_dev!\n");
> + goto err_register_netdev;
> + }
> +
> + netif_carrier_off(failover_dev);
> +
> + failover = net_failover_register(failover_dev, NULL);
> + if (IS_ERR(failover))
> + goto err_failover_register;
> +
> + return failover;
> +
> +err_failover_register:
> + unregister_netdev(failover_dev);
> +err_register_netdev:
> + free_netdev(failover_dev);
> +
> + return ERR_PTR(err);
> +}
> +EXPORT_SYMBOL_GPL(net_failover_create);
--
~Randy
^ permalink raw reply
* [PATCH net-next v10 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: Sridhar Samudrala @ 2018-05-07 22:10 UTC (permalink / raw)
To: mst, stephen, davem, netdev, virtualization, virtio-dev,
jesse.brandeburg, alexander.h.duyck, kubakici, sridhar.samudrala,
jasowang, loseweigh, jiri, aaron.f.brown
In-Reply-To: <1525731046-10989-1-git-send-email-sridhar.samudrala@intel.com>
Use the registration/notification framework supported by the generic
failover infrastructure.
Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
drivers/net/hyperv/Kconfig | 1 +
drivers/net/hyperv/hyperv_net.h | 2 +
drivers/net/hyperv/netvsc_drv.c | 134 +++++++---------------------------------
3 files changed, 26 insertions(+), 111 deletions(-)
diff --git a/drivers/net/hyperv/Kconfig b/drivers/net/hyperv/Kconfig
index 0765d5f61714..1f8419fc7c7f 100644
--- a/drivers/net/hyperv/Kconfig
+++ b/drivers/net/hyperv/Kconfig
@@ -2,5 +2,6 @@ config HYPERV_NET
tristate "Microsoft Hyper-V virtual network driver"
depends on HYPERV
select UCS2_STRING
+ select NET_FAILOVER
help
Select this option to enable the Hyper-V virtual network driver.
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6ebe39a3dde6..2ec18344c0e8 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -932,6 +932,8 @@ struct net_device_context {
u32 vf_alloc;
/* Serial number of the VF to team with */
u32 vf_serial;
+
+ struct net_failover *failover;
};
/* Per channel data */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index ecc84954c511..f852b3a9403a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -43,6 +43,7 @@
#include <net/pkt_sched.h>
#include <net/checksum.h>
#include <net/ip6_checksum.h>
+#include <net/net_failover.h>
#include "hyperv_net.h"
@@ -1763,46 +1764,6 @@ static void netvsc_link_change(struct work_struct *w)
rtnl_unlock();
}
-static struct net_device *get_netvsc_bymac(const u8 *mac)
-{
- struct net_device *dev;
-
- ASSERT_RTNL();
-
- for_each_netdev(&init_net, dev) {
- if (dev->netdev_ops != &device_ops)
- continue; /* not a netvsc device */
-
- if (ether_addr_equal(mac, dev->perm_addr))
- return dev;
- }
-
- return NULL;
-}
-
-static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
-{
- struct net_device *dev;
-
- ASSERT_RTNL();
-
- for_each_netdev(&init_net, dev) {
- struct net_device_context *net_device_ctx;
-
- if (dev->netdev_ops != &device_ops)
- continue; /* not a netvsc device */
-
- net_device_ctx = netdev_priv(dev);
- if (!rtnl_dereference(net_device_ctx->nvdev))
- continue; /* device is removed */
-
- if (rtnl_dereference(net_device_ctx->vf_netdev) == vf_netdev)
- return dev; /* a match */
- }
-
- return NULL;
-}
-
/* Called when VF is injecting data into network stack.
* Change the associated network device from VF to netvsc.
* note: already called with rcu_read_lock
@@ -1914,24 +1875,15 @@ static void netvsc_vf_setup(struct work_struct *w)
rtnl_unlock();
}
-static int netvsc_register_vf(struct net_device *vf_netdev)
+static int netvsc_register_vf(struct net_device *vf_netdev,
+ struct net_device *ndev)
{
- struct net_device *ndev;
struct net_device_context *net_device_ctx;
struct netvsc_device *netvsc_dev;
if (vf_netdev->addr_len != ETH_ALEN)
return NOTIFY_DONE;
- /*
- * We will use the MAC address to locate the synthetic interface to
- * associate with the VF interface. If we don't find a matching
- * synthetic interface, move on.
- */
- ndev = get_netvsc_bymac(vf_netdev->perm_addr);
- if (!ndev)
- return NOTIFY_DONE;
-
net_device_ctx = netdev_priv(ndev);
netvsc_dev = rtnl_dereference(net_device_ctx->nvdev);
if (!netvsc_dev || rtnl_dereference(net_device_ctx->vf_netdev))
@@ -1948,17 +1900,13 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
}
/* VF up/down change detected, schedule to change data path */
-static int netvsc_vf_changed(struct net_device *vf_netdev)
+static int netvsc_vf_changed(struct net_device *vf_netdev,
+ struct net_device *ndev)
{
struct net_device_context *net_device_ctx;
struct netvsc_device *netvsc_dev;
- struct net_device *ndev;
bool vf_is_up = netif_running(vf_netdev);
- ndev = get_netvsc_byref(vf_netdev);
- if (!ndev)
- return NOTIFY_DONE;
-
net_device_ctx = netdev_priv(ndev);
netvsc_dev = rtnl_dereference(net_device_ctx->nvdev);
if (!netvsc_dev)
@@ -1971,15 +1919,11 @@ static int netvsc_vf_changed(struct net_device *vf_netdev)
return NOTIFY_OK;
}
-static int netvsc_unregister_vf(struct net_device *vf_netdev)
+static int netvsc_unregister_vf(struct net_device *vf_netdev,
+ struct net_device *ndev)
{
- struct net_device *ndev;
struct net_device_context *net_device_ctx;
- ndev = get_netvsc_byref(vf_netdev);
- if (!ndev)
- return NOTIFY_DONE;
-
net_device_ctx = netdev_priv(ndev);
cancel_delayed_work_sync(&net_device_ctx->vf_takeover);
@@ -1993,6 +1937,12 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
return NOTIFY_OK;
}
+static struct net_failover_ops netvsc_failover_ops = {
+ .slave_register = netvsc_register_vf,
+ .slave_unregister = netvsc_unregister_vf,
+ .slave_link_change = netvsc_vf_changed,
+};
+
static int netvsc_probe(struct hv_device *dev,
const struct hv_vmbus_device_id *dev_id)
{
@@ -2082,8 +2032,15 @@ static int netvsc_probe(struct hv_device *dev,
goto register_failed;
}
+ net_device_ctx->failover = net_failover_register(net,
+ &netvsc_failover_ops);
+ if (IS_ERR(net_device_ctx->failover))
+ goto err_failover;
+
return ret;
+err_failover:
+ unregister_netdev(net);
register_failed:
rndis_filter_device_remove(dev, nvdev);
rndis_failed:
@@ -2124,13 +2081,15 @@ static int netvsc_remove(struct hv_device *dev)
rtnl_lock();
vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
if (vf_netdev)
- netvsc_unregister_vf(vf_netdev);
+ net_failover_slave_unregister(vf_netdev);
if (nvdev)
rndis_filter_device_remove(dev, nvdev);
unregister_netdevice(net);
+ net_failover_unregister(ndev_ctx->failover);
+
rtnl_unlock();
rcu_read_unlock();
@@ -2157,54 +2116,8 @@ static struct hv_driver netvsc_drv = {
.remove = netvsc_remove,
};
-/*
- * On Hyper-V, every VF interface is matched with a corresponding
- * synthetic interface. The synthetic interface is presented first
- * to the guest. When the corresponding VF instance is registered,
- * we will take care of switching the data path.
- */
-static int netvsc_netdev_event(struct notifier_block *this,
- unsigned long event, void *ptr)
-{
- struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
-
- /* Skip our own events */
- if (event_dev->netdev_ops == &device_ops)
- return NOTIFY_DONE;
-
- /* Avoid non-Ethernet type devices */
- if (event_dev->type != ARPHRD_ETHER)
- return NOTIFY_DONE;
-
- /* Avoid Vlan dev with same MAC registering as VF */
- if (is_vlan_dev(event_dev))
- return NOTIFY_DONE;
-
- /* Avoid Bonding master dev with same MAC registering as VF */
- if ((event_dev->priv_flags & IFF_BONDING) &&
- (event_dev->flags & IFF_MASTER))
- return NOTIFY_DONE;
-
- switch (event) {
- case NETDEV_REGISTER:
- return netvsc_register_vf(event_dev);
- case NETDEV_UNREGISTER:
- return netvsc_unregister_vf(event_dev);
- case NETDEV_UP:
- case NETDEV_DOWN:
- return netvsc_vf_changed(event_dev);
- default:
- return NOTIFY_DONE;
- }
-}
-
-static struct notifier_block netvsc_netdev_notifier = {
- .notifier_call = netvsc_netdev_event,
-};
-
static void __exit netvsc_drv_exit(void)
{
- unregister_netdevice_notifier(&netvsc_netdev_notifier);
vmbus_driver_unregister(&netvsc_drv);
}
@@ -2224,7 +2137,6 @@ static int __init netvsc_drv_init(void)
if (ret)
return ret;
- register_netdevice_notifier(&netvsc_netdev_notifier);
return 0;
}
--
2.14.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox