> -----Original Message-----
> From: Aaron Conole <[email protected] <mailto:[email protected]>>
> Date: Wednesday 22 February 2023 at 18:11
> To: "[email protected] <mailto:[email protected]>" 
> <[email protected] <mailto:[email protected]>>
> Cc: Eli Britstein <[email protected] <mailto:[email protected]>>, Gaetan Rivet 
> <[email protected] <mailto:[email protected]>>, Ilya Maximets 
> <[email protected] <mailto:[email protected]>>, Maxime Coquelin 
> <[email protected] <mailto:[email protected]>>, Jason  
> Gunthorpe <[email protected] <mailto:[email protected]>>, Majd Dibbiny 
> <[email protected] <mailto:[email protected]>>, David Marchand 
> <[email protected] <mailto:[email protected]>>
> Subject: Re: [ovs-dev] [RFC] dpdk: Allow retaining cap_sys_rawio privileges
>
> Apologies - I mis-typed Gaetan's email when I entered it into my mail
> file. CC'd correctly on this email (but I can resend the patch, if you
> think it is better).
>
> Aaron Conole <[email protected] <mailto:[email protected]>> writes:
>
> > Open vSwitch generally tries to let the underlying operating system
> > managed the low level details of hardware, for example DMA mapping,
> > bus arbitration, etc. However, when using DPDK, the underlying
> > operating system yields control of many of these details to userspace
> > for management.
> >
> > In the case of some DPDK port drivers, configuring rte_flow or even
> > allocating resources may require access to iopl/ioperm calls, which
> > are guarded by the CAP_SYS_RAWIO privilege on linux systems. These
> > calls are dangerous, and can allow a process to completely compromise
> > a system. However, they are needed in the case of some userspace
> > driver code which manages the hardware (for example, the mlx
> > implementation of backend support for rte_flow).
> >
> > Here, we create an opt-in flag passed to the command line to allow
> > this access. We need to do this before ever accessing the database,
> > because we want to drop all privileges asap, and cannot wait for
> > a connection to the database to be established and functional before
> > dropping. There may be distribution specific ways to do capability
> > management as well (using for example, systemd), but they are not
> > as universal to the vswitchd as a flag.
> >
> > Signed-off-by: Aaron Conole <[email protected] <mailto:[email protected]>>
> > ---

Hello Aaron,

Thank you for proposing this change.

If users want to use mlx5 ports with OVS without being root, this capability 
will be required.
Adding a vswitchd option to enable it seems the simplest way to offer some 
control.

If vendor-specific logic was allowed, I could add a function to detect Mellanox 
ports
and enable this option in that case. Otherwise we can document as much as 
possible,
but hopefully the errors will be made clear from the DPDK side because it will
be hard to explain those errors without vendor-specific code.

Regarding the implementation, I had a few comments:
> @@ -877,11 +890,11 @@ daemon_become_new_user__(bool access_datapath)
>   * However, there in case the user switch needs to be done
>   * before daemonize_start(), the following API can be used.  */
>  void
> -daemon_become_new_user(bool access_datapath)
> +daemon_become_new_user(bool access_datapath, bool access_hardware_ports)
>  {
>      assert_single_threaded();
>      if (switch_user) {
> -        daemon_become_new_user__(access_datapath);
> +        daemon_become_new_user__(access_datapath, access_hardware_ports);
>          /* daemonize_start() should not switch user again. */
>          switch_user = false;
>      }

Grepping for daemon_become_new_user, I see the following that might need a 
change:
lib/daemon-windows.c:529:daemon_become_new_user(bool access_datapath OVS_UNUSED)

> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index 407bfc60e..f62d1ad75 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -60,6 +60,9 @@ VLOG_DEFINE_THIS_MODULE(vswitchd);
>   * the kernel from paging any of its memory to disk. */
>  static bool want_mlockall;
>
> +/* --hw-access: If set, retains CAP_SYS_RAWIO privileges.  */
> +static bool hw_access;
> +
>  static unixctl_cb_func ovs_vswitchd_exit;
>
>  static char *parse_options(int argc, char *argv[], char **unixctl_path);
> @@ -89,7 +92,7 @@ main(int argc, char *argv[])
>      remote = parse_options(argc, argv, &unixctl_path);
>      fatal_ignore_sigpipe();
>
> -    daemonize_start(true);
> +    daemonize_start(true, true);
 
Here I think it should be daemonize_start(true, hw_access);

Best regards,

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to