Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Rich Persaud]

> On Dec 16, 2019, at 03:31, JÃrgen Groà <jgross@xxxxxxxx> wrote:
> 
> ïOn 16.12.19 09:18, Durrant, Paul wrote:
>>> -----Original Message-----
>>> From: JÃrgen Groà <jgross@xxxxxxxx>
>>> Sent: 16 December 2019 08:10
>>> To: Durrant, Paul <pdurrant@xxxxxxxxxx>; David Miller
>>> <davem@xxxxxxxxxxxxx>
>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; wei.liu@xxxxxxxxxx; linux-
>>> kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>>> related code
>>>> On 13.12.19 11:12, Durrant, Paul wrote:
>>>>>> -----Original Message-----
>>>>>> From: JÃrgen Groà <jgross@xxxxxxxx>
>>>>>> Sent: 13 December 2019 10:02
>>>>>> To: Durrant, Paul <pdurrant@xxxxxxxxxx>; David Miller
>>>>>> <davem@xxxxxxxxxxxxx>
>>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; wei.liu@xxxxxxxxxx; linux-
>>>>>> kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>> udev
>>>>> related code
>>>>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: JÃrgen Groà <jgross@xxxxxxxx>
>>>>>>> Sent: 13 December 2019 05:41
>>>>>>> To: David Miller <davem@xxxxxxxxxxxxx>; Durrant, Paul
>>>>>>> <pdurrant@xxxxxxxxxx>
>>>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; wei.liu@xxxxxxxxxx; linux-
>>>>>>> kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
>>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>>>> udev
>>>>>>> related code
>>>>>>>> On 12.12.19 20:05, David Miller wrote:
>>>>>>>>> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>>>>>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>>>>>> In the past it used to be the case that the Xen toolstack relied
>>>> upon
>>>>>>>>>> udev to execute backend hotplug scripts. However this has not been
>>>>>> the
>>>>>>>>>> case for many releases now and removal of the associated code in
>>>>>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>>>>>> much
>>>>>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>>>>>> uevent()
>>>>>>>>>>         method. The only other driver to have a method at all is
>>>>>>>>>>         pvcalls-back, and currently pvcalls_back_uevent() simply
>>>>>> returns
>>>>>>>> 0.
>>>>>>>>>>         Hence this patch also facilitates further cleanup.
>>>>>>>>>> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
>>>>>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>>>>>> this
>>>>>>>>> even if it hasn't been used in 5+ years.
>>>>>>>> Hmm, depends.
>>>>>>>> This has been used by Xen tools in dom0 only. If the last usage has
>>>>>> been
>>>>>>>> in a Xen version which is no longer able to run with current Linux in
>>>>>>>> dom0 it could be removed. But I guess this would have to be a rather
>>>>>> old
>>>>>>>> version of Xen (like 3.x?).
>>>>>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>>>>>> longer relies on udev to start the hotplug scripts?
>>>>>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>>>>>> backend.rules (in xen.git), and a commit from Roger removed the NIC
>>>> rules
>>>>>> in 2012:
>>>>>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>>>>> Xen 4.2
>>>>>>> The last commit I could find to that file modified its name to xen-
>>>>>> backend.rules.in, and this was finally removed by George in 2015:
>>>>>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>>>>> Xen 4.6
>>>>>>> So, I think this means anyone using a version of the Xen tools within
>>>>>> recent memory will be having their hotplug scripts called directly by
>>>>>> libxl (and having udev rules present would actually be counter-
>>>> productive,
>>>>>> as George's commit states and as I discovered the hard way when the
>>>> change
>>>>>> was originally made).
>>>>>> The problem are systems with either old Xen versions (before Xen 4.2)
>>>> or
>>>>>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>>>>>> dom0 kernel.
>>>>>> And I'm not sure there aren't such systems (especially in case someone
>>>>>> wants to stick with xend).
>>>>> But would someone sticking with such an old toolstack expect to run on
>>>> an unmodified upstream dom0? There has to be some way in which we can
>>>> retire old code.
>>>> As long as there are no hypervisor interface related issues
>>>> prohibiting running dom0 unmodified I think the expectation to be
>>>> able to use the kernel in that environment is fine.
>> I think we need a better policy in future then otherwise we will only collect baggage.
> 
> The Linux kernel policy regarding user interfaces and existing use cases
> is rather clear and we should not deviate without very strong reasons.
> 
>>> Another question coming up would be: how is this handled in a driver
>>> domain running netback? Which component is starting the hotplug script
>>> there? I don't think we can assume a standard Xen toolset in this case.
>>> So I'd rather leave this code as it is instead of breaking some rare
>>> but valid use cases.
>> I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.
> 
> I intend in no way to break projects like Qubes. Disaggregation is
> one of the very big advantages of Xen over KVM, Hyper-V and VMWare.
> We should not give that up "just to get rid of some code". Period.

From a quick poll of Qubes and OpenXT netback usage: neither project would be impacted by the proposed change.  Qubes uses xl devd to call hotplug scripts.  OpenXT uses uevent triggers, but does not rely on any script provided in the uevent.  Ongoing simplification of backend drivers would be welcome.

Rich

> 
>>>> Aside from the udev kicks though, I still think the hotplug-status/ring
>>> state interaction is just bogus anyway. As I said in a previous thread,
>>> the hotplug-status ought to be indicated as carrier status, if at all, so
>>> I still think all that code ought to go.
>>> I agree regarding the future interface, but with the carrier state just
>>> being in the plans to be added now, it is clearly too early to remove
>>> the code with that reasoning.
>> I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.
> 
> If it is being used that way we need to continue supporting it.
> 
> 
> Juergen
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel