Jason Wang
2021-Jan-15 05:38 UTC
[PATCH linux-next v3 6/6] vdpa_sim_net: Add support for user supported devices
On 2021/1/14 ??3:58, Parav Pandit wrote:> >> From: Jason Wang <jasowang at redhat.com> >> Sent: Thursday, January 14, 2021 9:48 AM >> >> On 2021/1/7 ??11:48, Parav Pandit wrote: >>>> From: Michael S. Tsirkin <mst at redhat.com> >>>> Sent: Tuesday, January 5, 2021 6:53 PM >>>> >>>> On Tue, Jan 05, 2021 at 12:30:15PM +0000, Parav Pandit wrote: >>>>>> From: Michael S. Tsirkin <mst at redhat.com> >>>>>> Sent: Tuesday, January 5, 2021 5:45 PM >>>>>> >>>>>> On Tue, Jan 05, 2021 at 12:02:33PM +0000, Parav Pandit wrote: >>>>>>>> From: Michael S. Tsirkin <mst at redhat.com> >>>>>>>> Sent: Tuesday, January 5, 2021 5:19 PM >>>>>>>> >>>>>>>> On Tue, Jan 05, 2021 at 12:32:03PM +0200, Parav Pandit wrote: >>>>>>>>> Enable user to create vdpasim net simulate devices. >>>>>>>>> >>>>>>>>> >>>>>>>>> $ vdpa dev add mgmtdev vdpasim_net name foo2 >>>>>>>>> >>>>>>>>> Show the newly created vdpa device by its name: >>>>>>>>> $ vdpa dev show foo2 >>>>>>>>> foo2: type network mgmtdev vdpasim_net vendor_id 0 max_vqs 2 >>>>>>>>> max_vq_size 256 >>>>>>>>> >>>>>>>>> $ vdpa dev show foo2 -jp >>>>>>>>> { >>>>>>>>> "dev": { >>>>>>>>> "foo2": { >>>>>>>>> "type": "network", >>>>>>>>> "mgmtdev": "vdpasim_net", >>>>>>>>> "vendor_id": 0, >>>>>>>>> "max_vqs": 2, >>>>>>>>> "max_vq_size": 256 >>>>>>>>> } >>>>>>>>> } >>>>>>>>> } >>>>>>>> I'd like an example of how do device specific (e.g. net >>>>>>>> specific) interfaces tie in to this. >>>>>>> Not sure I follow your question. >>>>>>> Do you mean how to set mac address or mtu of this vdpa device of >>>>>>> type >>>>>> net? >>>>>>> If so, dev add command will be extended shortly in subsequent >>>>>>> series to >>>>>> set this net specific attributes. >>>>>>> (I did mention in the next steps in cover letter). >>>>>>> >>>>>>>>> +static int __init vdpasim_net_init(void) { >>>>>>>>> + int ret; >>>>>>>>> + >>>>>>>>> + if (macaddr) { >>>>>>>>> + mac_pton(macaddr, macaddr_buf); >>>>>>>>> + if (!is_valid_ether_addr(macaddr_buf)) >>>>>>>>> + return -EADDRNOTAVAIL; >>>>>>>>> + } else { >>>>>>>>> + eth_random_addr(macaddr_buf); >>>>>>>>> } >>>>>>>> Hmm so all devices start out with the same MAC until changed? >>>>>>>> And how is the change effected? >>>>>>> Post this patchset and post we have iproute2 vdpa in the tree, >>>>>>> will add the >>>>>> mac address as the input attribute during "vdpa dev add" command. >>>>>>> So that each different vdpa device can have user specified >>>>>>> (different) mac >>>>>> address. >>>>>> >>>>>> For now maybe just avoid VIRTIO_NET_F_MAC then for new devices >>>> then? >>>>> That would require book keeping existing net vdpa_sim devices >>>>> created to >>>> avoid setting VIRTIO_NET_F_MAC. >>>>> Such book keeping code will be short lived anyway. >>>>> Not sure if its worth it. >>>>> Until now only one device was created. So not sure two vdpa devices >>>>> with >>>> same mac address will be a real issue. >>>>> When we add mac address attribute in add command, at that point also >>>> remove the module parameter macaddr. >>>> >>>> Will that be mandatory? I'm not to happy with a UAPI we intend to >>>> break straight away ... >>> No. Specifying mac address shouldn't be mandatory. UAPI wont' be >> broken. >> >> >> If it's not mandatory. Does it mean the vDPA parent need to use its own logic >> to generate a validate mac? I'm not sure this is what management (libvirt >> want). >> > There are few use cases that I see with PFs, VFs and SFs supporting vdpa devices. > > 1. User wants to use the VF only for vdpa purpose. Here user got the VF which was pre-setup by the sysadmin. > In this case whatever MAC assigned to the VF can be used by its vdpa device. > Here, user doesn't need to pass the mac address during vdpa device creation time. > This is done as the same MAC has been setup in the ACL rules on the switch side. > Non VDPA users of a VF typically use the VF this way for Netdev and rdma functionality. > They might continue same way for vdpa application as well. > Here VF mac is either set using > (a) devlink port function set hw_addr command or using > (b) ip link set vf mac > So vdpa tool didn't pass the mac. (optional). > Though VIRTIO_NET_F_MAC is still valid. > > 2. User may want to create one or more vdpa device out of the mgmt. device. > Here user wants to more/full control of all features, overriding what sysadmin has setup as MAC of the VF/SF. > In this case user will specify the MAC via mgmt tool. > (a) This is also used by those vdpa devices which doesn't have eswitch offloads. > (b) This will work with eswitch offloads as well who does source learning. > (c) User chose to use the vdpa device of a VF while VF Netdev and rdma device are used by hypervisor for something else as well. > VIRTIO_NET_F_MAC remains valid in all 2.{a,b,c}. > > 3. A vendor mgmt. device always expects it user to provide mac for its vdpa devices. > So when it is not provided, it can fail with error message string in extack or clear the VIRTIO_NET_F_MAC and let it work using virtio spec's 5.1.5 point 5 to proceed. > > As common denominator of all above cases, if QEMU or user pass the MAC during creation, it will almost always work. > Advance user and QEMU with switchdev mode support who has done 1.a/1.b, will omit it. > I do not know how deep integration of QEMU exist with the switchdev mode support. > > With that mac, mtu as optional input fields provide the necessary flexibility for different stacks to take appropriate shape as they desire.Thanks for the clarification. I think we'd better document the above in the patch that introduces the mac setting from management API.>
Parav Pandit
2021-Jan-15 06:27 UTC
[PATCH linux-next v3 6/6] vdpa_sim_net: Add support for user supported devices
> From: Jason Wang <jasowang at redhat.com> > Sent: Friday, January 15, 2021 11:09 AM > > > On 2021/1/14 ??3:58, Parav Pandit wrote: > > > >> From: Jason Wang <jasowang at redhat.com> > >> Sent: Thursday, January 14, 2021 9:48 AM > >> > >> On 2021/1/7 ??11:48, Parav Pandit wrote: > >>>> From: Michael S. Tsirkin <mst at redhat.com> > >>>> Sent: Tuesday, January 5, 2021 6:53 PM > >>>> > >>>> On Tue, Jan 05, 2021 at 12:30:15PM +0000, Parav Pandit wrote: > >>>>>> From: Michael S. Tsirkin <mst at redhat.com> > >>>>>> Sent: Tuesday, January 5, 2021 5:45 PM > >>>>>> > >>>>>> On Tue, Jan 05, 2021 at 12:02:33PM +0000, Parav Pandit wrote: > >>>>>>>> From: Michael S. Tsirkin <mst at redhat.com> > >>>>>>>> Sent: Tuesday, January 5, 2021 5:19 PM > >>>>>>>> > >>>>>>>> On Tue, Jan 05, 2021 at 12:32:03PM +0200, Parav Pandit wrote: > >>>>>>>>> Enable user to create vdpasim net simulate devices. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> $ vdpa dev add mgmtdev vdpasim_net name foo2 > >>>>>>>>> > >>>>>>>>> Show the newly created vdpa device by its name: > >>>>>>>>> $ vdpa dev show foo2 > >>>>>>>>> foo2: type network mgmtdev vdpasim_net vendor_id 0 > max_vqs 2 > >>>>>>>>> max_vq_size 256 > >>>>>>>>> > >>>>>>>>> $ vdpa dev show foo2 -jp > >>>>>>>>> { > >>>>>>>>> "dev": { > >>>>>>>>> "foo2": { > >>>>>>>>> "type": "network", > >>>>>>>>> "mgmtdev": "vdpasim_net", > >>>>>>>>> "vendor_id": 0, > >>>>>>>>> "max_vqs": 2, > >>>>>>>>> "max_vq_size": 256 > >>>>>>>>> } > >>>>>>>>> } > >>>>>>>>> } > >>>>>>>> I'd like an example of how do device specific (e.g. net > >>>>>>>> specific) interfaces tie in to this. > >>>>>>> Not sure I follow your question. > >>>>>>> Do you mean how to set mac address or mtu of this vdpa device of > >>>>>>> type > >>>>>> net? > >>>>>>> If so, dev add command will be extended shortly in subsequent > >>>>>>> series to > >>>>>> set this net specific attributes. > >>>>>>> (I did mention in the next steps in cover letter). > >>>>>>> > >>>>>>>>> +static int __init vdpasim_net_init(void) { > >>>>>>>>> + int ret; > >>>>>>>>> + > >>>>>>>>> + if (macaddr) { > >>>>>>>>> + mac_pton(macaddr, macaddr_buf); > >>>>>>>>> + if (!is_valid_ether_addr(macaddr_buf)) > >>>>>>>>> + return -EADDRNOTAVAIL; > >>>>>>>>> + } else { > >>>>>>>>> + eth_random_addr(macaddr_buf); > >>>>>>>>> } > >>>>>>>> Hmm so all devices start out with the same MAC until changed? > >>>>>>>> And how is the change effected? > >>>>>>> Post this patchset and post we have iproute2 vdpa in the tree, > >>>>>>> will add the > >>>>>> mac address as the input attribute during "vdpa dev add" command. > >>>>>>> So that each different vdpa device can have user specified > >>>>>>> (different) mac > >>>>>> address. > >>>>>> > >>>>>> For now maybe just avoid VIRTIO_NET_F_MAC then for new devices > >>>> then? > >>>>> That would require book keeping existing net vdpa_sim devices > >>>>> created to > >>>> avoid setting VIRTIO_NET_F_MAC. > >>>>> Such book keeping code will be short lived anyway. > >>>>> Not sure if its worth it. > >>>>> Until now only one device was created. So not sure two vdpa > >>>>> devices with > >>>> same mac address will be a real issue. > >>>>> When we add mac address attribute in add command, at that point > >>>>> also > >>>> remove the module parameter macaddr. > >>>> > >>>> Will that be mandatory? I'm not to happy with a UAPI we intend to > >>>> break straight away ... > >>> No. Specifying mac address shouldn't be mandatory. UAPI wont' be > >> broken. > >> > >> > >> If it's not mandatory. Does it mean the vDPA parent need to use its > >> own logic to generate a validate mac? I'm not sure this is what > >> management (libvirt want). > >> > > There are few use cases that I see with PFs, VFs and SFs supporting vdpa > devices. > > > > 1. User wants to use the VF only for vdpa purpose. Here user got the VF > which was pre-setup by the sysadmin. > > In this case whatever MAC assigned to the VF can be used by its vdpa > device. > > Here, user doesn't need to pass the mac address during vdpa device > creation time. > > This is done as the same MAC has been setup in the ACL rules on the switch > side. > > Non VDPA users of a VF typically use the VF this way for Netdev and rdma > functionality. > > They might continue same way for vdpa application as well. > > Here VF mac is either set using > > (a) devlink port function set hw_addr command or using > > (b) ip link set vf mac > > So vdpa tool didn't pass the mac. (optional). > > Though VIRTIO_NET_F_MAC is still valid. > > > > 2. User may want to create one or more vdpa device out of the mgmt. > device. > > Here user wants to more/full control of all features, overriding what > sysadmin has setup as MAC of the VF/SF. > > In this case user will specify the MAC via mgmt tool. > > (a) This is also used by those vdpa devices which doesn't have eswitch > offloads. > > (b) This will work with eswitch offloads as well who does source learning. > > (c) User chose to use the vdpa device of a VF while VF Netdev and rdma > device are used by hypervisor for something else as well. > > VIRTIO_NET_F_MAC remains valid in all 2.{a,b,c}. > > > > 3. A vendor mgmt. device always expects it user to provide mac for its > vdpa devices. > > So when it is not provided, it can fail with error message string in extack or > clear the VIRTIO_NET_F_MAC and let it work using virtio spec's 5.1.5 point 5 > to proceed. > > > > As common denominator of all above cases, if QEMU or user pass the MAC > during creation, it will almost always work. > > Advance user and QEMU with switchdev mode support who has done > 1.a/1.b, will omit it. > > I do not know how deep integration of QEMU exist with the switchdev > mode support. > > > > With that mac, mtu as optional input fields provide the necessary flexibility > for different stacks to take appropriate shape as they desire. > > > Thanks for the clarification. I think we'd better document the above in the > patch that introduces the mac setting from management API.Yes. Will do. Thanks.
Parav Pandit
2021-Jan-18 18:03 UTC
[PATCH linux-next v3 6/6] vdpa_sim_net: Add support for user supported devices
Hi Michael, Jason,> From: Jason Wang <jasowang at redhat.com> > Sent: Friday, January 15, 2021 11:09 AM > > > Thanks for the clarification. I think we'd better document the above in the > patch that introduces the mac setting from management API.Can we proceed with this patchset? We like to progress next to iproute2/vdpa, mac and other drivers post this series in this kernel version.