patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 1/4] power: make guest channel headers public
       [not found] <20200814110045.217724-1-bruce.richardson@intel.com>
@ 2020-08-14 11:00 ` Bruce Richardson
  2020-08-14 11:05   ` Bruce Richardson
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2020-08-14 11:00 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, David Hunt, Pablo de Lara, Alan Carew

The channel commands header file contains definitions that are used by the
example application for power management, and so need to be made public.
Similarly the guest_channel.h header is used by the guest_cli example
utility, so needs to be public also.  Without this change, the example
apps, or any end-user apps based on them, can not be compiled outside the
main DPDK build.

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/vm_power_manager/channel_manager.c                   | 1 -
 examples/vm_power_manager/channel_monitor.c                   | 1 -
 examples/vm_power_manager/channel_monitor.h                   | 2 +-
 examples/vm_power_manager/guest_cli/main.c                    | 1 +
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.c      | 2 +-
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.h      | 2 +-
 examples/vm_power_manager/vm_power_cli.c                      | 1 -
 lib/librte_power/guest_channel.c                              | 3 +--
 lib/librte_power/meson.build                                  | 4 +++-
 lib/librte_power/power_kvm_vm.c                               | 3 +--
 .../{channel_commands.h => rte_power_channel_commands.h}      | 0
 .../{guest_channel.h => rte_power_guest_channel.h}            | 2 +-
 12 files changed, 10 insertions(+), 12 deletions(-)
 rename lib/librte_power/{channel_commands.h => rte_power_channel_commands.h} (100%)
 rename lib/librte_power/{guest_channel.h => rte_power_guest_channel.h} (98%)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 74a2a677e8..3da01b46d8 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -27,7 +27,6 @@
 #include <libvirt/libvirt.h>
 
 #include "channel_manager.h"
-#include "channel_commands.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
 
diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 1d00a6cf6c..75a29d2589 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -35,7 +35,6 @@
 
 #include <libvirt/libvirt.h>
 #include "channel_monitor.h"
-#include "channel_commands.h"
 #include "channel_manager.h"
 #include "power_manager.h"
 #include "oob_monitor.h"
diff --git a/examples/vm_power_manager/channel_monitor.h b/examples/vm_power_manager/channel_monitor.h
index 7362a80d26..fe6ceedc95 100644
--- a/examples/vm_power_manager/channel_monitor.h
+++ b/examples/vm_power_manager/channel_monitor.h
@@ -6,7 +6,7 @@
 #define CHANNEL_MONITOR_H_
 
 #include "channel_manager.h"
-#include "channel_commands.h"
+#include "rte_power_channel_commands.h"
 
 struct core_share {
 	unsigned int pcpu;
diff --git a/examples/vm_power_manager/guest_cli/main.c b/examples/vm_power_manager/guest_cli/main.c
index f63b3c988a..4c0c98f498 100644
--- a/examples/vm_power_manager/guest_cli/main.c
+++ b/examples/vm_power_manager/guest_cli/main.c
@@ -14,6 +14,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 
+#include "rte_power_channel_commands.h"
 #include "vm_power_cli_guest.h"
 #include "parse.h"
 
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 96c1a1ff69..263e7ec9ff 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -19,7 +19,7 @@
 #include <rte_ethdev.h>
 
 #include <rte_power.h>
-#include <guest_channel.h>
+#include <rte_power_guest_channel.h>
 
 #include "vm_power_cli_guest.h"
 
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
index 6ad14a3dea..33c01ff7ca 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
@@ -9,7 +9,7 @@
 extern "C" {
 #endif
 
-#include "channel_commands.h"
+struct channel_packet;
 
 struct channel_packet *get_policy(void);
 
diff --git a/examples/vm_power_manager/vm_power_cli.c b/examples/vm_power_manager/vm_power_cli.c
index 7edeaccda5..4c41645664 100644
--- a/examples/vm_power_manager/vm_power_cli.c
+++ b/examples/vm_power_manager/vm_power_cli.c
@@ -21,7 +21,6 @@
 #include "channel_manager.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
-#include "channel_commands.h"
 
 struct cmd_quit_result {
 	cmdline_fixed_string_t quit;
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 7b5926e5c4..d49cf0cd57 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -16,8 +16,7 @@
 
 #include <rte_log.h>
 
-#include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
diff --git a/lib/librte_power/meson.build b/lib/librte_power/meson.build
index 78c031c943..e9b1fa6e45 100644
--- a/lib/librte_power/meson.build
+++ b/lib/librte_power/meson.build
@@ -10,5 +10,7 @@ sources = files('rte_power.c', 'power_acpi_cpufreq.c',
 		'rte_power_empty_poll.c',
 		'power_pstate_cpufreq.c',
 		'power_common.c')
-headers = files('rte_power.h','rte_power_empty_poll.h')
+headers = files('rte_power.h','rte_power_empty_poll.h',
+		'rte_power_guest_channel.h',
+		'rte_power_channel_commands.h')
 deps += ['timer']
diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
index 409c3e03ab..e9094e1230 100644
--- a/lib/librte_power/power_kvm_vm.c
+++ b/lib/librte_power/power_kvm_vm.c
@@ -6,8 +6,7 @@
 
 #include <rte_log.h>
 
-#include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 #include "power_kvm_vm.h"
 #include "power_common.h"
 
diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/rte_power_channel_commands.h
similarity index 100%
rename from lib/librte_power/channel_commands.h
rename to lib/librte_power/rte_power_channel_commands.h
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/rte_power_guest_channel.h
similarity index 98%
rename from lib/librte_power/guest_channel.h
rename to lib/librte_power/rte_power_guest_channel.h
index e15db46fc7..9853839129 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/rte_power_guest_channel.h
@@ -8,7 +8,7 @@
 extern "C" {
 #endif
 
-#include <channel_commands.h>
+#include <rte_power_channel_commands.h>
 
 /**
  * Check if any Virtio-Serial VM end-points exist in path.
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH 2/4] examples/vm_power_manager: fix string truncation warning
       [not found] <20200814110045.217724-1-bruce.richardson@intel.com>
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 1/4] power: make guest channel headers public Bruce Richardson
@ 2020-08-14 11:00 ` Bruce Richardson
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-08-14 11:00 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, David Hunt, Alan Carew, Pablo de Lara

When compiling on ubuntu 20.04, a warning was issued about possible
truncation of the path string for the power management socket.

channel_manager.c: In function ‘add_all_channels’:
channel_manager.c:470:41: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 90 [-Wformat-truncation=]
  470 |     sizeof(chan_info->channel_path), "%s%s",
      |                                         ^~

This can be fixed by adding in an explicit truncation check to the code and
handling it appropriately.

Fixes: e8ae9b662506 ("examples/vm_power: channel manager and monitor in host")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/vm_power_manager/channel_manager.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 3da01b46d8..0a28cb643b 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -466,9 +466,15 @@ add_all_channels(const char *vm_name)
 			continue;
 		}
 
-		snprintf(chan_info->channel_path,
+		if ((size_t)snprintf(chan_info->channel_path,
 				sizeof(chan_info->channel_path), "%s%s",
-				CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+				CHANNEL_MGR_SOCKET_PATH, dir->d_name)
+					>= sizeof(chan_info->channel_path)) {
+			RTE_LOG(ERR, CHANNEL_MANAGER, "Pathname too long for channel '%s%s'\n",
+					CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+			rte_free(chan_info);
+			continue;
+		}
 
 		if (setup_channel_info(&vm_info, &chan_info, channel_num) < 0) {
 			rte_free(chan_info);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH 3/4] examples/mp_server: fix snprintf overflow
       [not found] <20200814110045.217724-1-bruce.richardson@intel.com>
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 1/4] power: make guest channel headers public Bruce Richardson
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
@ 2020-08-14 11:00 ` Bruce Richardson
  2020-08-14 15:01   ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2020-08-14 11:00 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Anatoly Burakov

When producing a printable mac address the buffer was appropriately sized
for holding the mac address exactly, but the actual snprintf included a
'\n' character at the end, which means that the snprintf technically is
getting truncated i.e. the \n would not be added due to lack of space.
This gets flagged as a problem by modern versions of gcc, e.g. on Ubuntu
20.04.

main.c:77:37: warning: ‘__builtin___snprintf_chk’ output truncated before the last format character [-Wformat-truncation=]
   77 |     "%02x:%02x:%02x:%02x:%02x:%02x\n",
      |                                     ^

Since the \n is getting stripped anyway, we can fix the issue by just
removing it from the printf string.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/multi_process/client_server_mp/mp_server/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/multi_process/client_server_mp/mp_server/main.c b/examples/multi_process/client_server_mp/mp_server/main.c
index 280dab8672..af5af672c3 100644
--- a/examples/multi_process/client_server_mp/mp_server/main.c
+++ b/examples/multi_process/client_server_mp/mp_server/main.c
@@ -74,7 +74,7 @@ get_printable_mac_addr(uint16_t port)
 			return err_address;
 		}
 		snprintf(addresses[port], sizeof(addresses[port]),
-				"%02x:%02x:%02x:%02x:%02x:%02x\n",
+				"%02x:%02x:%02x:%02x:%02x:%02x",
 				mac.addr_bytes[0], mac.addr_bytes[1], mac.addr_bytes[2],
 				mac.addr_bytes[3], mac.addr_bytes[4], mac.addr_bytes[5]);
 	}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH 4/4] examples/mp_server: clear string truncation warning
       [not found] <20200814110045.217724-1-bruce.richardson@intel.com>
                   ` (2 preceding siblings ...)
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
@ 2020-08-14 11:00 ` Bruce Richardson
       [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
       [not found] ` <20201028162702.969509-1-bruce.richardson@intel.com>
  5 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-08-14 11:00 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Anatoly Burakov

Compiling with GCC 9.3 on Ubuntu 20.04 gives a warning about possible
string truncation when getting the RX queue name:

In file included from init.c:36:
init.c: In function ‘init’:
../shared/common.h:38:28: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
   38 | #define MP_CLIENT_RXQ_NAME "MProc_Client_%u_RX"
      |                            ^~~~~~~~~~~~~~~~~~~~
../shared/common.h:52:35: note: in expansion of macro ‘MP_CLIENT_RXQ_NAME’
   52 |  snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id);
      |                                   ^~~~~~~~~~~~~~~~~~

This is a false positive, as the value of the "id" is limited to 255, being
stored in the app as a uint8_t value, removing the possibility of the %u
being replaced by anything other then 3 characters max (rather than up to
10 as thought by the compiler). Therefore, the warning can be easily
removed by changing the type of the "id" parameter to the local function
from "unsigned" to "uint8_t" also, ensuring the compiler is aware of the
range limit.

Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/multi_process/client_server_mp/shared/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/multi_process/client_server_mp/shared/common.h b/examples/multi_process/client_server_mp/shared/common.h
index 6dd43fcac2..76beca0101 100644
--- a/examples/multi_process/client_server_mp/shared/common.h
+++ b/examples/multi_process/client_server_mp/shared/common.h
@@ -43,7 +43,7 @@ struct port_info {
  * Given the rx queue name template above, get the queue name
  */
 static inline const char *
-get_rx_queue_name(unsigned id)
+get_rx_queue_name(uint8_t id)
 {
 	/* buffer for return value. Size calculated by %u being replaced
 	 * by maximum 3 digits (plus an extra byte for safety) */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [PATCH 1/4] power: make guest channel headers public
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 1/4] power: make guest channel headers public Bruce Richardson
@ 2020-08-14 11:05   ` Bruce Richardson
  0 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-08-14 11:05 UTC (permalink / raw)
  To: dev; +Cc: stable, David Hunt, Pablo de Lara, Alan Carew

On Fri, Aug 14, 2020 at 12:00:42PM +0100, Bruce Richardson wrote:
> The channel commands header file contains definitions that are used by the
> example application for power management, and so need to be made public.
> Similarly the guest_channel.h header is used by the guest_cli example
> utility, so needs to be public also.  Without this change, the example
> apps, or any end-user apps based on them, can not be compiled outside the
> main DPDK build.
> 
> Fixes: 210c383e247b ("power: packet format for vm power management")
> Fixes: cd0d5547e873 ("power: vm communication channels in guest")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/vm_power_manager/channel_manager.c                   | 1 -
>  examples/vm_power_manager/channel_monitor.c                   | 1 -
>  examples/vm_power_manager/channel_monitor.h                   | 2 +-
>  examples/vm_power_manager/guest_cli/main.c                    | 1 +
>  examples/vm_power_manager/guest_cli/vm_power_cli_guest.c      | 2 +-
>  examples/vm_power_manager/guest_cli/vm_power_cli_guest.h      | 2 +-
>  examples/vm_power_manager/vm_power_cli.c                      | 1 -
>  lib/librte_power/guest_channel.c                              | 3 +--
>  lib/librte_power/meson.build                                  | 4 +++-
>  lib/librte_power/power_kvm_vm.c                               | 3 +--
>  .../{channel_commands.h => rte_power_channel_commands.h}      | 0
>  .../{guest_channel.h => rte_power_guest_channel.h}            | 2 +-
>  12 files changed, 10 insertions(+), 12 deletions(-)
>  rename lib/librte_power/{channel_commands.h => rte_power_channel_commands.h} (100%)
>  rename lib/librte_power/{guest_channel.h => rte_power_guest_channel.h} (98%)
> 
Note, this patch does not include the makefile changes, as the work was
done when testing with the makefile removal patchset applied, and make
support is being removed in this release.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH 3/4] examples/mp_server: fix snprintf overflow
  2020-08-14 11:00 ` [dpdk-stable] [PATCH 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
@ 2020-08-14 15:01   ` Stephen Hemminger
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2020-08-14 15:01 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, stable, Anatoly Burakov

On Fri, 14 Aug 2020 12:00:44 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> When producing a printable mac address the buffer was appropriately sized
> for holding the mac address exactly, but the actual snprintf included a
> '\n' character at the end, which means that the snprintf technically is
> getting truncated i.e. the \n would not be added due to lack of space.
> This gets flagged as a problem by modern versions of gcc, e.g. on Ubuntu
> 20.04.
> 
> main.c:77:37: warning: ‘__builtin___snprintf_chk’ output truncated before the last format character [-Wformat-truncation=]
>    77 |     "%02x:%02x:%02x:%02x:%02x:%02x\n",
>       |                                     ^
> 
> Since the \n is getting stripped anyway, we can fix the issue by just
> removing it from the printf string.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/multi_process/client_server_mp/mp_server/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/multi_process/client_server_mp/mp_server/main.c b/examples/multi_process/client_server_mp/mp_server/main.c
> index 280dab8672..af5af672c3 100644
> --- a/examples/multi_process/client_server_mp/mp_server/main.c
> +++ b/examples/multi_process/client_server_mp/mp_server/main.c
> @@ -74,7 +74,7 @@ get_printable_mac_addr(uint16_t port)
>  			return err_address;
>  		}
>  		snprintf(addresses[port], sizeof(addresses[port]),
> -				"%02x:%02x:%02x:%02x:%02x:%02x\n",
> +				"%02x:%02x:%02x:%02x:%02x:%02x",
>  				mac.addr_bytes[0], mac.addr_bytes[1], mac.addr_bytes[2],
>  				mac.addr_bytes[3], mac.addr_bytes[4], mac.addr_bytes[5]);
>  	}

Please convert to rte_ether_format_addr() instead of local hard coded format.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public
       [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
@ 2020-08-21 17:10   ` Bruce Richardson
  2020-08-26 10:08     ` David Hunt
  2020-10-05 21:44     ` Thomas Monjalon
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-08-21 17:10 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, David Hunt, Pablo de Lara, Alan Carew

The channel commands header file contains definitions that are used by the
example application for power management, and so need to be made public.
Similarly the guest_channel.h header is used by the guest_cli example
utility, so needs to be public also.  Without this change, the example
apps, or any end-user apps based on them, can not be compiled outside the
main DPDK build.

Fixes: 210c383e247b ("power: packet format for vm power management")
Fixes: cd0d5547e873 ("power: vm communication channels in guest")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/vm_power_manager/channel_manager.c                   | 1 -
 examples/vm_power_manager/channel_monitor.c                   | 1 -
 examples/vm_power_manager/channel_monitor.h                   | 2 +-
 examples/vm_power_manager/guest_cli/main.c                    | 1 +
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.c      | 2 +-
 examples/vm_power_manager/guest_cli/vm_power_cli_guest.h      | 2 +-
 examples/vm_power_manager/vm_power_cli.c                      | 1 -
 lib/librte_power/guest_channel.c                              | 3 +--
 lib/librte_power/meson.build                                  | 4 +++-
 lib/librte_power/power_kvm_vm.c                               | 3 +--
 .../{channel_commands.h => rte_power_channel_commands.h}      | 0
 .../{guest_channel.h => rte_power_guest_channel.h}            | 2 +-
 12 files changed, 10 insertions(+), 12 deletions(-)
 rename lib/librte_power/{channel_commands.h => rte_power_channel_commands.h} (100%)
 rename lib/librte_power/{guest_channel.h => rte_power_guest_channel.h} (98%)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 74a2a677e8..3da01b46d8 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -27,7 +27,6 @@
 #include <libvirt/libvirt.h>
 
 #include "channel_manager.h"
-#include "channel_commands.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
 
diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 1d00a6cf6c..75a29d2589 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -35,7 +35,6 @@
 
 #include <libvirt/libvirt.h>
 #include "channel_monitor.h"
-#include "channel_commands.h"
 #include "channel_manager.h"
 #include "power_manager.h"
 #include "oob_monitor.h"
diff --git a/examples/vm_power_manager/channel_monitor.h b/examples/vm_power_manager/channel_monitor.h
index 7362a80d26..fe6ceedc95 100644
--- a/examples/vm_power_manager/channel_monitor.h
+++ b/examples/vm_power_manager/channel_monitor.h
@@ -6,7 +6,7 @@
 #define CHANNEL_MONITOR_H_
 
 #include "channel_manager.h"
-#include "channel_commands.h"
+#include "rte_power_channel_commands.h"
 
 struct core_share {
 	unsigned int pcpu;
diff --git a/examples/vm_power_manager/guest_cli/main.c b/examples/vm_power_manager/guest_cli/main.c
index f63b3c988a..4c0c98f498 100644
--- a/examples/vm_power_manager/guest_cli/main.c
+++ b/examples/vm_power_manager/guest_cli/main.c
@@ -14,6 +14,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 
+#include "rte_power_channel_commands.h"
 #include "vm_power_cli_guest.h"
 #include "parse.h"
 
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 96c1a1ff69..263e7ec9ff 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -19,7 +19,7 @@
 #include <rte_ethdev.h>
 
 #include <rte_power.h>
-#include <guest_channel.h>
+#include <rte_power_guest_channel.h>
 
 #include "vm_power_cli_guest.h"
 
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
index 6ad14a3dea..33c01ff7ca 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
@@ -9,7 +9,7 @@
 extern "C" {
 #endif
 
-#include "channel_commands.h"
+struct channel_packet;
 
 struct channel_packet *get_policy(void);
 
diff --git a/examples/vm_power_manager/vm_power_cli.c b/examples/vm_power_manager/vm_power_cli.c
index 7edeaccda5..4c41645664 100644
--- a/examples/vm_power_manager/vm_power_cli.c
+++ b/examples/vm_power_manager/vm_power_cli.c
@@ -21,7 +21,6 @@
 #include "channel_manager.h"
 #include "channel_monitor.h"
 #include "power_manager.h"
-#include "channel_commands.h"
 
 struct cmd_quit_result {
 	cmdline_fixed_string_t quit;
diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
index 7b5926e5c4..d49cf0cd57 100644
--- a/lib/librte_power/guest_channel.c
+++ b/lib/librte_power/guest_channel.c
@@ -16,8 +16,7 @@
 
 #include <rte_log.h>
 
-#include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 
 #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
 
diff --git a/lib/librte_power/meson.build b/lib/librte_power/meson.build
index 78c031c943..e9b1fa6e45 100644
--- a/lib/librte_power/meson.build
+++ b/lib/librte_power/meson.build
@@ -10,5 +10,7 @@ sources = files('rte_power.c', 'power_acpi_cpufreq.c',
 		'rte_power_empty_poll.c',
 		'power_pstate_cpufreq.c',
 		'power_common.c')
-headers = files('rte_power.h','rte_power_empty_poll.h')
+headers = files('rte_power.h','rte_power_empty_poll.h',
+		'rte_power_guest_channel.h',
+		'rte_power_channel_commands.h')
 deps += ['timer']
diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
index 409c3e03ab..e9094e1230 100644
--- a/lib/librte_power/power_kvm_vm.c
+++ b/lib/librte_power/power_kvm_vm.c
@@ -6,8 +6,7 @@
 
 #include <rte_log.h>
 
-#include "guest_channel.h"
-#include "channel_commands.h"
+#include "rte_power_guest_channel.h"
 #include "power_kvm_vm.h"
 #include "power_common.h"
 
diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/rte_power_channel_commands.h
similarity index 100%
rename from lib/librte_power/channel_commands.h
rename to lib/librte_power/rte_power_channel_commands.h
diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/rte_power_guest_channel.h
similarity index 98%
rename from lib/librte_power/guest_channel.h
rename to lib/librte_power/rte_power_guest_channel.h
index e15db46fc7..9853839129 100644
--- a/lib/librte_power/guest_channel.h
+++ b/lib/librte_power/rte_power_guest_channel.h
@@ -8,7 +8,7 @@
 extern "C" {
 #endif
 
-#include <channel_commands.h>
+#include <rte_power_channel_commands.h>
 
 /**
  * Check if any Virtio-Serial VM end-points exist in path.
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning
       [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public Bruce Richardson
@ 2020-08-21 17:10   ` Bruce Richardson
  2020-08-26 10:09     ` David Hunt
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
  3 siblings, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2020-08-21 17:10 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, David Hunt, Pablo de Lara, Alan Carew

When compiling on ubuntu 20.04, a warning was issued about possible
truncation of the path string for the power management socket.

channel_manager.c: In function ‘add_all_channels’:
channel_manager.c:470:41: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 90 [-Wformat-truncation=]
  470 |     sizeof(chan_info->channel_path), "%s%s",
      |                                         ^~

This can be fixed by adding in an explicit truncation check to the code and
handling it appropriately.

Fixes: e8ae9b662506 ("examples/vm_power: channel manager and monitor in host")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/vm_power_manager/channel_manager.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 3da01b46d8..0a28cb643b 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -466,9 +466,15 @@ add_all_channels(const char *vm_name)
 			continue;
 		}
 
-		snprintf(chan_info->channel_path,
+		if ((size_t)snprintf(chan_info->channel_path,
 				sizeof(chan_info->channel_path), "%s%s",
-				CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+				CHANNEL_MGR_SOCKET_PATH, dir->d_name)
+					>= sizeof(chan_info->channel_path)) {
+			RTE_LOG(ERR, CHANNEL_MANAGER, "Pathname too long for channel '%s%s'\n",
+					CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+			rte_free(chan_info);
+			continue;
+		}
 
 		if (setup_channel_info(&vm_info, &chan_info, channel_num) < 0) {
 			rte_free(chan_info);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow
       [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public Bruce Richardson
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
@ 2020-08-21 17:10   ` Bruce Richardson
  2020-10-09 11:22     ` [dpdk-stable] [dpdk-dev] " Nicolau, Radu
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
  3 siblings, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2020-08-21 17:10 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Stephen Hemminger, Anatoly Burakov

When producing a printable mac address the buffer was appropriately sized
for holding the mac address exactly, but the actual snprintf included a
'\n' character at the end, which means that the snprintf technically is
getting truncated i.e. the \n would not be added due to lack of space.
This gets flagged as a problem by modern versions of gcc, e.g. on Ubuntu
20.04.

main.c:77:37: warning: ‘__builtin___snprintf_chk’ output truncated before the last format character [-Wformat-truncation=]
   77 |     "%02x:%02x:%02x:%02x:%02x:%02x\n",
      |                                     ^

Since the \n is getting stripped anyway, we can fix the issue by just
removing it. In the process we can switch to using the standard ethernet
address formating function from rte_ether.h.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
Cc: Stephen Hemminger <stephen@networkplumber.org>

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
V2: switched code to use standard formatting function
---
 .../client_server_mp/mp_server/main.c           | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_server/main.c b/examples/multi_process/client_server_mp/mp_server/main.c
index 280dab8672..fb2aa49678 100644
--- a/examples/multi_process/client_server_mp/mp_server/main.c
+++ b/examples/multi_process/client_server_mp/mp_server/main.c
@@ -59,12 +59,17 @@ static struct client_rx_buf *cl_rx_buf;
 static const char *
 get_printable_mac_addr(uint16_t port)
 {
-	static const char err_address[] = "00:00:00:00:00:00";
-	static char addresses[RTE_MAX_ETHPORTS][sizeof(err_address)];
+	static const struct rte_ether_addr null_mac; /* static defaults to 0 */
+	static char err_address[32];
+	static char addresses[RTE_MAX_ETHPORTS][32];
 	int ret;
 
-	if (unlikely(port >= RTE_MAX_ETHPORTS))
+	if (unlikely(port >= RTE_MAX_ETHPORTS)) {
+		if (err_address[0] == '\0')
+			rte_ether_format_addr(err_address,
+					sizeof(err_address), &null_mac);
 		return err_address;
+	}
 	if (unlikely(addresses[port][0]=='\0')){
 		struct rte_ether_addr mac;
 		ret = rte_eth_macaddr_get(port, &mac);
@@ -73,10 +78,8 @@ get_printable_mac_addr(uint16_t port)
 			       port, rte_strerror(-ret));
 			return err_address;
 		}
-		snprintf(addresses[port], sizeof(addresses[port]),
-				"%02x:%02x:%02x:%02x:%02x:%02x\n",
-				mac.addr_bytes[0], mac.addr_bytes[1], mac.addr_bytes[2],
-				mac.addr_bytes[3], mac.addr_bytes[4], mac.addr_bytes[5]);
+		rte_ether_format_addr(addresses[port],
+				sizeof(addresses[port]), &mac);
 	}
 	return addresses[port];
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v2 4/4] examples/mp_server: clear string truncation warning
       [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
                     ` (2 preceding siblings ...)
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
@ 2020-08-21 17:10   ` Bruce Richardson
  2020-10-09 11:21     ` [dpdk-stable] [dpdk-dev] " Nicolau, Radu
  3 siblings, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2020-08-21 17:10 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Anatoly Burakov

Compiling with GCC 9.3 on Ubuntu 20.04 gives a warning about possible
string truncation when getting the RX queue name:

In file included from init.c:36:
init.c: In function ‘init’:
../shared/common.h:38:28: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
   38 | #define MP_CLIENT_RXQ_NAME "MProc_Client_%u_RX"
      |                            ^~~~~~~~~~~~~~~~~~~~
../shared/common.h:52:35: note: in expansion of macro ‘MP_CLIENT_RXQ_NAME’
   52 |  snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id);
      |                                   ^~~~~~~~~~~~~~~~~~

This is a false positive, as the value of the "id" is limited to 255, being
stored in the app as a uint8_t value, removing the possibility of the %u
being replaced by anything other then 3 characters max (rather than up to
10 as thought by the compiler). Therefore, the warning can be easily
removed by changing the type of the "id" parameter to the local function
from "unsigned" to "uint8_t" also, ensuring the compiler is aware of the
range limit.

Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/multi_process/client_server_mp/shared/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/multi_process/client_server_mp/shared/common.h b/examples/multi_process/client_server_mp/shared/common.h
index 6dd43fcac2..76beca0101 100644
--- a/examples/multi_process/client_server_mp/shared/common.h
+++ b/examples/multi_process/client_server_mp/shared/common.h
@@ -43,7 +43,7 @@ struct port_info {
  * Given the rx queue name template above, get the queue name
  */
 static inline const char *
-get_rx_queue_name(unsigned id)
+get_rx_queue_name(uint8_t id)
 {
 	/* buffer for return value. Size calculated by %u being replaced
 	 * by maximum 3 digits (plus an extra byte for safety) */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public Bruce Richardson
@ 2020-08-26 10:08     ` David Hunt
  2020-10-05 21:44     ` Thomas Monjalon
  1 sibling, 0 replies; 18+ messages in thread
From: David Hunt @ 2020-08-26 10:08 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Pablo de Lara, Alan Carew

Hi Bruce,

On 21/8/2020 6:10 PM, Bruce Richardson wrote:
> The channel commands header file contains definitions that are used by the
> example application for power management, and so need to be made public.
> Similarly the guest_channel.h header is used by the guest_cli example
> utility, so needs to be public also.  Without this change, the example
> apps, or any end-user apps based on them, can not be compiled outside the
> main DPDK build.
>
> Fixes: 210c383e247b ("power: packet format for vm power management")
> Fixes: cd0d5547e873 ("power: vm communication channels in guest")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   examples/vm_power_manager/channel_manager.c                   | 1 -
>   examples/vm_power_manager/channel_monitor.c                   | 1 -
>   examples/vm_power_manager/channel_monitor.h                   | 2 +-
>   examples/vm_power_manager/guest_cli/main.c                    | 1 +
>   examples/vm_power_manager/guest_cli/vm_power_cli_guest.c      | 2 +-
>   examples/vm_power_manager/guest_cli/vm_power_cli_guest.h      | 2 +-
>   examples/vm_power_manager/vm_power_cli.c                      | 1 -
>   lib/librte_power/guest_channel.c                              | 3 +--
>   lib/librte_power/meson.build                                  | 4 +++-
>   lib/librte_power/power_kvm_vm.c                               | 3 +--
>   .../{channel_commands.h => rte_power_channel_commands.h}      | 0
>   .../{guest_channel.h => rte_power_guest_channel.h}            | 2 +-
>   12 files changed, 10 insertions(+), 12 deletions(-)
>   rename lib/librte_power/{channel_commands.h => rte_power_channel_commands.h} (100%)
>   rename lib/librte_power/{guest_channel.h => rte_power_guest_channel.h} (98%)
>
> diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
> index 74a2a677e8..3da01b46d8 100644
> --- a/examples/vm_power_manager/channel_manager.c
> +++ b/examples/vm_power_manager/channel_manager.c
> @@ -27,7 +27,6 @@
>   #include <libvirt/libvirt.h>
>   
>   #include "channel_manager.h"
> -#include "channel_commands.h"
>   #include "channel_monitor.h"
>   #include "power_manager.h"
>   
> diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
> index 1d00a6cf6c..75a29d2589 100644
> --- a/examples/vm_power_manager/channel_monitor.c
> +++ b/examples/vm_power_manager/channel_monitor.c
> @@ -35,7 +35,6 @@
>   
>   #include <libvirt/libvirt.h>
>   #include "channel_monitor.h"
> -#include "channel_commands.h"
>   #include "channel_manager.h"
>   #include "power_manager.h"
>   #include "oob_monitor.h"
> diff --git a/examples/vm_power_manager/channel_monitor.h b/examples/vm_power_manager/channel_monitor.h
> index 7362a80d26..fe6ceedc95 100644
> --- a/examples/vm_power_manager/channel_monitor.h
> +++ b/examples/vm_power_manager/channel_monitor.h
> @@ -6,7 +6,7 @@
>   #define CHANNEL_MONITOR_H_
>   
>   #include "channel_manager.h"
> -#include "channel_commands.h"
> +#include "rte_power_channel_commands.h"
>   
>   struct core_share {
>   	unsigned int pcpu;
> diff --git a/examples/vm_power_manager/guest_cli/main.c b/examples/vm_power_manager/guest_cli/main.c
> index f63b3c988a..4c0c98f498 100644
> --- a/examples/vm_power_manager/guest_cli/main.c
> +++ b/examples/vm_power_manager/guest_cli/main.c
> @@ -14,6 +14,7 @@
>   #include <rte_eal.h>
>   #include <rte_log.h>
>   
> +#include "rte_power_channel_commands.h"
>   #include "vm_power_cli_guest.h"
>   #include "parse.h"
>   
> diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> index 96c1a1ff69..263e7ec9ff 100644
> --- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> +++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
> @@ -19,7 +19,7 @@
>   #include <rte_ethdev.h>
>   
>   #include <rte_power.h>
> -#include <guest_channel.h>
> +#include <rte_power_guest_channel.h>
>   
>   #include "vm_power_cli_guest.h"
>   
> diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
> index 6ad14a3dea..33c01ff7ca 100644
> --- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
> +++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.h
> @@ -9,7 +9,7 @@
>   extern "C" {
>   #endif
>   
> -#include "channel_commands.h"
> +struct channel_packet;
>   
>   struct channel_packet *get_policy(void);
>   
> diff --git a/examples/vm_power_manager/vm_power_cli.c b/examples/vm_power_manager/vm_power_cli.c
> index 7edeaccda5..4c41645664 100644
> --- a/examples/vm_power_manager/vm_power_cli.c
> +++ b/examples/vm_power_manager/vm_power_cli.c
> @@ -21,7 +21,6 @@
>   #include "channel_manager.h"
>   #include "channel_monitor.h"
>   #include "power_manager.h"
> -#include "channel_commands.h"
>   
>   struct cmd_quit_result {
>   	cmdline_fixed_string_t quit;
> diff --git a/lib/librte_power/guest_channel.c b/lib/librte_power/guest_channel.c
> index 7b5926e5c4..d49cf0cd57 100644
> --- a/lib/librte_power/guest_channel.c
> +++ b/lib/librte_power/guest_channel.c
> @@ -16,8 +16,7 @@
>   
>   #include <rte_log.h>
>   
> -#include "guest_channel.h"
> -#include "channel_commands.h"
> +#include "rte_power_guest_channel.h"
>   
>   #define RTE_LOGTYPE_GUEST_CHANNEL RTE_LOGTYPE_USER1
>   
> diff --git a/lib/librte_power/meson.build b/lib/librte_power/meson.build
> index 78c031c943..e9b1fa6e45 100644
> --- a/lib/librte_power/meson.build
> +++ b/lib/librte_power/meson.build
> @@ -10,5 +10,7 @@ sources = files('rte_power.c', 'power_acpi_cpufreq.c',
>   		'rte_power_empty_poll.c',
>   		'power_pstate_cpufreq.c',
>   		'power_common.c')
> -headers = files('rte_power.h','rte_power_empty_poll.h')
> +headers = files('rte_power.h','rte_power_empty_poll.h',
> +		'rte_power_guest_channel.h',
> +		'rte_power_channel_commands.h')
>   deps += ['timer']
> diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
> index 409c3e03ab..e9094e1230 100644
> --- a/lib/librte_power/power_kvm_vm.c
> +++ b/lib/librte_power/power_kvm_vm.c
> @@ -6,8 +6,7 @@
>   
>   #include <rte_log.h>
>   
> -#include "guest_channel.h"
> -#include "channel_commands.h"
> +#include "rte_power_guest_channel.h"
>   #include "power_kvm_vm.h"
>   #include "power_common.h"
>   
> diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/rte_power_channel_commands.h
> similarity index 100%
> rename from lib/librte_power/channel_commands.h
> rename to lib/librte_power/rte_power_channel_commands.h
> diff --git a/lib/librte_power/guest_channel.h b/lib/librte_power/rte_power_guest_channel.h
> similarity index 98%
> rename from lib/librte_power/guest_channel.h
> rename to lib/librte_power/rte_power_guest_channel.h
> index e15db46fc7..9853839129 100644
> --- a/lib/librte_power/guest_channel.h
> +++ b/lib/librte_power/rte_power_guest_channel.h
> @@ -8,7 +8,7 @@
>   extern "C" {
>   #endif
>   
> -#include <channel_commands.h>
> +#include <rte_power_channel_commands.h>
>   
>   /**
>    * Check if any Virtio-Serial VM end-points exist in path.


Acked-by: David Hunt <david.hunt@intel.com>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
@ 2020-08-26 10:09     ` David Hunt
  0 siblings, 0 replies; 18+ messages in thread
From: David Hunt @ 2020-08-26 10:09 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Pablo de Lara, Alan Carew

Hi Bruce,

On 21/8/2020 6:10 PM, Bruce Richardson wrote:
> When compiling on ubuntu 20.04, a warning was issued about possible
> truncation of the path string for the power management socket.
>
> channel_manager.c: In function ‘add_all_channels’:
> channel_manager.c:470:41: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 90 [-Wformat-truncation=]
>    470 |     sizeof(chan_info->channel_path), "%s%s",
>        |                                         ^~
>
> This can be fixed by adding in an explicit truncation check to the code and
> handling it appropriately.
>
> Fixes: e8ae9b662506 ("examples/vm_power: channel manager and monitor in host")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   examples/vm_power_manager/channel_manager.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
> index 3da01b46d8..0a28cb643b 100644
> --- a/examples/vm_power_manager/channel_manager.c
> +++ b/examples/vm_power_manager/channel_manager.c
> @@ -466,9 +466,15 @@ add_all_channels(const char *vm_name)
>   			continue;
>   		}
>   
> -		snprintf(chan_info->channel_path,
> +		if ((size_t)snprintf(chan_info->channel_path,
>   				sizeof(chan_info->channel_path), "%s%s",
> -				CHANNEL_MGR_SOCKET_PATH, dir->d_name);
> +				CHANNEL_MGR_SOCKET_PATH, dir->d_name)
> +					>= sizeof(chan_info->channel_path)) {
> +			RTE_LOG(ERR, CHANNEL_MANAGER, "Pathname too long for channel '%s%s'\n",
> +					CHANNEL_MGR_SOCKET_PATH, dir->d_name);
> +			rte_free(chan_info);
> +			continue;
> +		}
>   
>   		if (setup_channel_info(&vm_info, &chan_info, channel_num) < 0) {
>   			rte_free(chan_info);


Acked-by: David Hunt <david.hunt@intel.com>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public Bruce Richardson
  2020-08-26 10:08     ` David Hunt
@ 2020-10-05 21:44     ` Thomas Monjalon
  1 sibling, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2020-10-05 21:44 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, stable, David Hunt, Pablo de Lara, Alan Carew

\0> The channel commands header file contains definitions that are used by the
> example application for power management, and so need to be made public.
> Similarly the guest_channel.h header is used by the guest_cli example
> utility, so needs to be public also.  Without this change, the example
> apps, or any end-user apps based on them, can not be compiled outside the
> main DPDK build.
[...]
>  rename lib/librte_power/{channel_commands.h => rte_power_channel_commands.h} (100%)
>  rename lib/librte_power/{guest_channel.h => rte_power_guest_channel.h} (98%)

Yes there are dirty includes, but it requires more than just a renaming.
At least the namespace of these functions should be checked
to start with "rte_".
But before adding all these new functions as stable public API,
I would like we check whether it is a well designed API or not.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 4/4] examples/mp_server: clear string truncation warning
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
@ 2020-10-09 11:21     ` Nicolau, Radu
  0 siblings, 0 replies; 18+ messages in thread
From: Nicolau, Radu @ 2020-10-09 11:21 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Anatoly Burakov


On 8/21/2020 6:10 PM, Bruce Richardson wrote:
> Compiling with GCC 9.3 on Ubuntu 20.04 gives a warning about possible
> string truncation when getting the RX queue name:
>
> In file included from init.c:36:
> init.c: In function ‘init’:
> ../shared/common.h:38:28: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
>     38 | #define MP_CLIENT_RXQ_NAME "MProc_Client_%u_RX"
>        |                            ^~~~~~~~~~~~~~~~~~~~
> ../shared/common.h:52:35: note: in expansion of macro ‘MP_CLIENT_RXQ_NAME’
>     52 |  snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id);
>        |                                   ^~~~~~~~~~~~~~~~~~
>
> This is a false positive, as the value of the "id" is limited to 255, being
> stored in the app as a uint8_t value, removing the possibility of the %u
> being replaced by anything other then 3 characters max (rather than up to
> 10 as thought by the compiler). Therefore, the warning can be easily
> removed by changing the type of the "id" parameter to the local function
> from "unsigned" to "uint8_t" also, ensuring the compiler is aware of the
> range limit.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Acked-by: Radu Nicolau <radu.nicolau@intel.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow
  2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
@ 2020-10-09 11:22     ` Nicolau, Radu
  0 siblings, 0 replies; 18+ messages in thread
From: Nicolau, Radu @ 2020-10-09 11:22 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Stephen Hemminger, Anatoly Burakov


On 8/21/2020 6:10 PM, Bruce Richardson wrote:
> When producing a printable mac address the buffer was appropriately sized
> for holding the mac address exactly, but the actual snprintf included a
> '\n' character at the end, which means that the snprintf technically is
> getting truncated i.e. the \n would not be added due to lack of space.
> This gets flagged as a problem by modern versions of gcc, e.g. on Ubuntu
> 20.04.
>
> main.c:77:37: warning: ‘__builtin___snprintf_chk’ output truncated before the last format character [-Wformat-truncation=]
>     77 |     "%02x:%02x:%02x:%02x:%02x:%02x\n",
>        |                                     ^
>
> Since the \n is getting stripped anyway, we can fix the issue by just
> removing it. In the process we can switch to using the standard ethernet
> address formating function from rte_ether.h.
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> ---
> V2: switched code to use standard formatting function
> ---

Acked-by: Radu Nicolau <radu.nicolau@intel.com>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v3 1/3] examples/vm_power_manager: fix string truncation warning
       [not found] ` <20201028162702.969509-1-bruce.richardson@intel.com>
@ 2020-10-28 16:27   ` Bruce Richardson
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 2/3] examples/mp_server: fix snprintf overflow Bruce Richardson
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 3/3] examples/mp_server: clear string truncation warning Bruce Richardson
  2 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-10-28 16:27 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, David Hunt, Alan Carew, Pablo de Lara

When compiling on ubuntu 20.04, a warning was issued about possible
truncation of the path string for the power management socket.

channel_manager.c: In function ‘add_all_channels’:
channel_manager.c:470:41: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 90 [-Wformat-truncation=]
  470 |     sizeof(chan_info->channel_path), "%s%s",
      |                                         ^~

This can be fixed by adding in an explicit truncation check to the code and
handling it appropriately.

Fixes: e8ae9b662506 ("examples/vm_power: channel manager and monitor in host")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
---
 examples/vm_power_manager/channel_manager.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 74a2a677e..a26315051 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -467,9 +467,15 @@ add_all_channels(const char *vm_name)
 			continue;
 		}
 
-		snprintf(chan_info->channel_path,
+		if ((size_t)snprintf(chan_info->channel_path,
 				sizeof(chan_info->channel_path), "%s%s",
-				CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+				CHANNEL_MGR_SOCKET_PATH, dir->d_name)
+					>= sizeof(chan_info->channel_path)) {
+			RTE_LOG(ERR, CHANNEL_MANAGER, "Pathname too long for channel '%s%s'\n",
+					CHANNEL_MGR_SOCKET_PATH, dir->d_name);
+			rte_free(chan_info);
+			continue;
+		}
 
 		if (setup_channel_info(&vm_info, &chan_info, channel_num) < 0) {
 			rte_free(chan_info);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v3 2/3] examples/mp_server: fix snprintf overflow
       [not found] ` <20201028162702.969509-1-bruce.richardson@intel.com>
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 1/3] examples/vm_power_manager: fix " Bruce Richardson
@ 2020-10-28 16:27   ` Bruce Richardson
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 3/3] examples/mp_server: clear string truncation warning Bruce Richardson
  2 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-10-28 16:27 UTC (permalink / raw)
  To: dev
  Cc: Bruce Richardson, stable, Stephen Hemminger, Radu Nicolau,
	Anatoly Burakov

When producing a printable mac address the buffer was appropriately sized
for holding the mac address exactly, but the actual snprintf included a
'\n' character at the end, which means that the snprintf technically is
getting truncated i.e. the \n would not be added due to lack of space.
This gets flagged as a problem by modern versions of gcc, e.g. on Ubuntu
20.04.

main.c:77:37: warning: ‘__builtin___snprintf_chk’ output truncated before the last format character [-Wformat-truncation=]
   77 |     "%02x:%02x:%02x:%02x:%02x:%02x\n",
      |                                     ^

Since the \n is getting stripped anyway, we can fix the issue by just
removing it. In the process we can switch to using the standard ethernet
address formating function from rte_ether.h.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
Cc: Stephen Hemminger <stephen@networkplumber.org>

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
V2: switched code to use standard formatting function
---

 .../client_server_mp/mp_server/main.c           | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_server/main.c b/examples/multi_process/client_server_mp/mp_server/main.c
index ec7f6b11f..b18e12dd4 100644
--- a/examples/multi_process/client_server_mp/mp_server/main.c
+++ b/examples/multi_process/client_server_mp/mp_server/main.c
@@ -59,12 +59,17 @@ static struct client_rx_buf *cl_rx_buf;
 static const char *
 get_printable_mac_addr(uint16_t port)
 {
-	static const char err_address[] = "00:00:00:00:00:00";
-	static char addresses[RTE_MAX_ETHPORTS][sizeof(err_address)];
+	static const struct rte_ether_addr null_mac; /* static defaults to 0 */
+	static char err_address[32];
+	static char addresses[RTE_MAX_ETHPORTS][32];
 	int ret;
 
-	if (unlikely(port >= RTE_MAX_ETHPORTS))
+	if (unlikely(port >= RTE_MAX_ETHPORTS)) {
+		if (err_address[0] == '\0')
+			rte_ether_format_addr(err_address,
+					sizeof(err_address), &null_mac);
 		return err_address;
+	}
 	if (unlikely(addresses[port][0]=='\0')){
 		struct rte_ether_addr mac;
 		ret = rte_eth_macaddr_get(port, &mac);
@@ -73,10 +78,8 @@ get_printable_mac_addr(uint16_t port)
 			       port, rte_strerror(-ret));
 			return err_address;
 		}
-		snprintf(addresses[port], sizeof(addresses[port]),
-				"%02x:%02x:%02x:%02x:%02x:%02x\n",
-				mac.addr_bytes[0], mac.addr_bytes[1], mac.addr_bytes[2],
-				mac.addr_bytes[3], mac.addr_bytes[4], mac.addr_bytes[5]);
+		rte_ether_format_addr(addresses[port],
+				sizeof(addresses[port]), &mac);
 	}
 	return addresses[port];
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-stable] [PATCH v3 3/3] examples/mp_server: clear string truncation warning
       [not found] ` <20201028162702.969509-1-bruce.richardson@intel.com>
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 1/3] examples/vm_power_manager: fix " Bruce Richardson
  2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 2/3] examples/mp_server: fix snprintf overflow Bruce Richardson
@ 2020-10-28 16:27   ` Bruce Richardson
  2 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2020-10-28 16:27 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Radu Nicolau, Anatoly Burakov

Compiling with GCC 9.3 on Ubuntu 20.04 gives a warning about possible
string truncation when getting the RX queue name:

In file included from init.c:36:
init.c: In function ‘init’:
../shared/common.h:38:28: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
   38 | #define MP_CLIENT_RXQ_NAME "MProc_Client_%u_RX"
      |                            ^~~~~~~~~~~~~~~~~~~~
../shared/common.h:52:35: note: in expansion of macro ‘MP_CLIENT_RXQ_NAME’
   52 |  snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id);
      |                                   ^~~~~~~~~~~~~~~~~~

This is a false positive, as the value of the "id" is limited to 255, being
stored in the app as a uint8_t value, removing the possibility of the %u
being replaced by anything other then 3 characters max (rather than up to
10 as thought by the compiler). Therefore, the warning can be easily
removed by changing the type of the "id" parameter to the local function
from "unsigned" to "uint8_t" also, ensuring the compiler is aware of the
range limit.

Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
 examples/multi_process/client_server_mp/shared/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/multi_process/client_server_mp/shared/common.h b/examples/multi_process/client_server_mp/shared/common.h
index 6dd43fcac..76beca010 100644
--- a/examples/multi_process/client_server_mp/shared/common.h
+++ b/examples/multi_process/client_server_mp/shared/common.h
@@ -43,7 +43,7 @@ struct port_info {
  * Given the rx queue name template above, get the queue name
  */
 static inline const char *
-get_rx_queue_name(unsigned id)
+get_rx_queue_name(uint8_t id)
 {
 	/* buffer for return value. Size calculated by %u being replaced
 	 * by maximum 3 digits (plus an extra byte for safety) */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-10-28 16:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200814110045.217724-1-bruce.richardson@intel.com>
2020-08-14 11:00 ` [dpdk-stable] [PATCH 1/4] power: make guest channel headers public Bruce Richardson
2020-08-14 11:05   ` Bruce Richardson
2020-08-14 11:00 ` [dpdk-stable] [PATCH 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
2020-08-14 11:00 ` [dpdk-stable] [PATCH 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
2020-08-14 15:01   ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2020-08-14 11:00 ` [dpdk-stable] [PATCH 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
     [not found] ` <20200821171017.50531-1-bruce.richardson@intel.com>
2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 1/4] power: make guest channel headers public Bruce Richardson
2020-08-26 10:08     ` David Hunt
2020-10-05 21:44     ` Thomas Monjalon
2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 2/4] examples/vm_power_manager: fix string truncation warning Bruce Richardson
2020-08-26 10:09     ` David Hunt
2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 3/4] examples/mp_server: fix snprintf overflow Bruce Richardson
2020-10-09 11:22     ` [dpdk-stable] [dpdk-dev] " Nicolau, Radu
2020-08-21 17:10   ` [dpdk-stable] [PATCH v2 4/4] examples/mp_server: clear string truncation warning Bruce Richardson
2020-10-09 11:21     ` [dpdk-stable] [dpdk-dev] " Nicolau, Radu
     [not found] ` <20201028162702.969509-1-bruce.richardson@intel.com>
2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 1/3] examples/vm_power_manager: fix " Bruce Richardson
2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 2/3] examples/mp_server: fix snprintf overflow Bruce Richardson
2020-10-28 16:27   ` [dpdk-stable] [PATCH v3 3/3] examples/mp_server: clear string truncation warning Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).