Ilya Maximets <i.maxim...@ovn.org> writes:
> On 3/16/23 13:00, Aaron Conole wrote:
>> 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.
>>
>> Reviewed-by: Simon Horman <simon.hor...@corigine.com>
>> Signed-off-by: Aaron Conole <acon...@redhat.com>
>> ---
>> NEWS | 4 ++++
>> lib/daemon-unix.c | 31 +++++++++++++++++++++++--------
>> lib/daemon-windows.c | 6 ++++--
>> lib/daemon.c | 2 +-
>> lib/daemon.h | 4 ++--
>> ovsdb/ovsdb-client.c | 6 +++---
>> ovsdb/ovsdb-server.c | 4 ++--
>> tests/test-netflow.c | 2 +-
>> tests/test-sflow.c | 2 +-
>> tests/test-unixctl.c | 2 +-
>> utilities/ovs-ofctl.c | 4 ++--
>> utilities/ovs-testcontroller.c | 4 ++--
>> vswitchd/ovs-vswitchd.8.in | 9 +++++++++
>> vswitchd/ovs-vswitchd.c | 11 ++++++++++-
>> 14 files changed, 65 insertions(+), 26 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 72b9024e6d..8771ee618a 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -17,6 +17,10 @@ Post-v3.1.0
>> in order to create OVSDB sockets with access mode of 0770.
>> - QoS:
>> * Added new configuration option 'jitter' for a linux-netem QoS type.
>> + - DPDK:
>> + * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
>> + with the --hw-rawio-access command line option. This allows the
>> + process extra privileges when mapping physical interconnect memory.
>>
>>
>> v3.1.0 - 16 Feb 2023
>> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
>> index 1a7ba427d7..5ec75106bc 100644
>> --- a/lib/daemon-unix.c
>> +++ b/lib/daemon-unix.c
>> @@ -88,7 +88,8 @@ static bool switch_user = false;
>> static uid_t uid;
>> static gid_t gid;
>> static char *user = NULL;
>> -static void daemon_become_new_user__(bool access_datapath);
>> +static void daemon_become_new_user__(bool access_datapath,
>> + bool access_hardware_ports);
>>
>> static void check_already_running(void);
>> static int lock_pidfile(FILE *, int command);
>> @@ -443,13 +444,13 @@ monitor_daemon(pid_t daemon_pid)
>> * daemonize_complete()) or that it failed to start up (by exiting with a
>> * nonzero exit code). */
>> void
>> -daemonize_start(bool access_datapath)
>> +daemonize_start(bool access_datapath, bool access_hardware_ports)
>> {
>> assert_single_threaded();
>> daemonize_fd = -1;
>>
>> if (switch_user) {
>> - daemon_become_new_user__(access_datapath);
>> + daemon_become_new_user__(access_datapath, access_hardware_ports);
>> switch_user = false;
>> }
>>
>> @@ -807,7 +808,8 @@ daemon_become_new_user_unix(void)
>> /* Linux specific implementation of daemon_become_new_user()
>> * using libcap-ng. */
>> static void
>> -daemon_become_new_user_linux(bool access_datapath OVS_UNUSED)
>> +daemon_become_new_user_linux(bool access_datapath OVS_UNUSED,
>> + bool access_hardware_ports OVS_UNUSED)
>> {
>> #if defined __linux__ && HAVE_LIBCAPNG
>> int ret;
>> @@ -827,6 +829,18 @@ daemon_become_new_user_linux(bool access_datapath
>> OVS_UNUSED)
>> ret = capng_update(CAPNG_ADD, cap_sets, CAP_NET_ADMIN)
>> || capng_update(CAPNG_ADD, cap_sets, CAP_NET_RAW)
>> || capng_update(CAPNG_ADD, cap_sets,
>> CAP_NET_BROADCAST);
>> +#ifdef DPDK_NETDEV
>> + if (access_hardware_ports && !ret) {
>> + ret = capng_update(CAPNG_ADD, cap_sets, CAP_SYS_RAWIO);
>> + VLOG_INFO("The Linux capability CAP_SYS_RAWIO "
>> + "is enabled.");
>
> I think, we shouldn't really log if the capng_update() failed.
> I could wrap the log in if (ret) {} while applying the change,
> if that makes sense. What do you think?
ACK
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
