DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] net/tap: avoid using SIGIO
@ 2020-04-22  1:01 Stephen Hemminger
  2020-07-14 23:58 ` [dpdk-dev] [PATCH] " Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-04-22  1:01 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The TAP device uses Linux signal internally to wakeup.
The use of SIGIO is problematic because it maybe used by application.
Instead choose another rt-signal since Linux allows any signal to be
used for signal based IO.
Search for an unused signal in the available rt-signal range.

Add more error checking for fcntl and signal handling.

Note: TAP is unique to Linux, BSD and Windows based TAP devices
have different API's so the whole driver will need major rework
if it ever used there.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 99 ++++++++++++++++++++++++-----------
 1 file changed, 67 insertions(+), 32 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 05470a211588..c71bb19801df 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -134,7 +134,7 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
 #ifdef IFF_MULTI_QUEUE
 	unsigned int features;
 #endif
-	int fd;
+	int fd, signo, flags;
 
 	memset(&ifr, 0, sizeof(struct ifreq));
 
@@ -199,52 +199,87 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
 		}
 	}
 
+	flags = fcntl(fd, F_GETFL);
+	if (flags == -1) {
+		TAP_LOG(WARNING,
+			"Unable to get %s current flags\n",
+			ifr.ifr_name);
+		goto error;
+	}
+
 	/* Always set the file descriptor to non-blocking */
-	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
+	flags |= O_NONBLOCK;
+	if (fcntl(fd, F_SETFL, flags) < 0) {
 		TAP_LOG(WARNING,
 			"Unable to set %s to nonblocking: %s",
 			ifr.ifr_name, strerror(errno));
 		goto error;
 	}
 
-	/* Set up trigger to optimize empty Rx bursts */
-	errno = 0;
-	do {
+	/* Find a free realtime signal */
+	for (signo = SIGRTMIN + 1; signo < SIGRTMAX; signo++) {
 		struct sigaction sa;
-		int flags = fcntl(fd, F_GETFL);
 
-		if (flags == -1 || sigaction(SIGIO, NULL, &sa) == -1)
+		if (sigaction(signo, NULL, &sa) == -1) {
+			TAP_LOG(WARNING,
+				"Unable to get current rt-signal %d handler",
+				signo);
+			goto error;
+		}
+
+		/* Already have the handler we want on this signal  */
+		if (sa.sa_handler == tap_trigger_cb)
 			break;
-		if (sa.sa_handler != tap_trigger_cb) {
-			/*
-			 * Make sure SIGIO is not already taken. This is done
-			 * as late as possible to leave the application a
-			 * chance to set up its own signal handler first.
-			 */
-			if (sa.sa_handler != SIG_IGN &&
-			    sa.sa_handler != SIG_DFL) {
-				errno = EBUSY;
-				break;
-			}
-			sa = (struct sigaction){
-				.sa_flags = SA_RESTART,
-				.sa_handler = tap_trigger_cb,
-			};
-			if (sigaction(SIGIO, &sa, NULL) == -1)
-				break;
+
+		/* Is handler in use by application */
+		if (sa.sa_handler != SIG_DFL) {
+			TAP_LOG(DEBUG,
+				"Skipping used rt-signal %d", signo);
+			continue;
+		}
+
+		sa = (struct sigaction) {
+			.sa_flags = SA_RESTART,
+			.sa_handler = tap_trigger_cb,
+		};
+
+		if (sigaction(signo, &sa, NULL) == -1) {
+			TAP_LOG(WARNING,
+				"Unable to set rt-signal %d handler\n", signo);
+			goto error;
 		}
-		/* Enable SIGIO on file descriptor */
-		fcntl(fd, F_SETFL, flags | O_ASYNC);
-		fcntl(fd, F_SETOWN, getpid());
-	} while (0);
 
-	if (errno) {
+		/* Found a good signal to use */
+		TAP_LOG(DEBUG,
+			"Using rt-signal %d", signo);
+		break;
+	}
+
+	if (signo == SIGRTMAX) {
+		TAP_LOG(WARNING, "All rt-signals are in use\n");
+
 		/* Disable trigger globally in case of error */
 		tap_trigger = 0;
-		TAP_LOG(WARNING, "Rx trigger disabled: %s",
-			strerror(errno));
-	}
+		TAP_LOG(NOTICE, "No Rx trigger signal available\n");
+	} else {
+		/* Enable signal on file descriptor */
+		if (fcntl(fd, F_SETSIG, signo) < 0) {
+			TAP_LOG(WARNING, "Unable to set signo %d for fd %d: %s",
+				signo, fd, strerror(errno));
+			goto error;
+		}
+		if (fcntl(fd, F_SETFL, flags | O_ASYNC) < 0) {
+			TAP_LOG(WARNING, "Unable to set fcntl flags: %s",
+				strerror(errno));
+			goto error;
+		}
 
+		if (fcntl(fd, F_SETOWN, getpid()) < 0) {
+			TAP_LOG(WARNING, "Unable to set fcntl owner: %s",
+				strerror(errno));
+			goto error;
+		}
+	}
 	return fd;
 
 error:
-- 
2.20.1


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

* [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-04-22  1:01 [dpdk-dev] [RFC] net/tap: avoid using SIGIO Stephen Hemminger
@ 2020-07-14 23:58 ` Stephen Hemminger
  2020-07-27 13:19   ` Morten Brørup
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-07-14 23:58 UTC (permalink / raw)
  To: keith.wiles; +Cc: dev, Stephen Hemminger

SIGIO maybe used by application, instead choose another rt-signal.
Linux allows any signal to be used for signal based IO.
Search for an unused signal in the available rt-signal range.

Add more error checking for fcntl and signal handling.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Resending original version, marked as PATCH now.

 drivers/net/tap/rte_eth_tap.c | 99 ++++++++++++++++++++++++-----------
 1 file changed, 67 insertions(+), 32 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 339f24bf82f3..b19e26ba0e65 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -134,7 +134,7 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
 #ifdef IFF_MULTI_QUEUE
 	unsigned int features;
 #endif
-	int fd;
+	int fd, signo, flags;
 
 	memset(&ifr, 0, sizeof(struct ifreq));
 
@@ -199,52 +199,87 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
 		}
 	}
 
+	flags = fcntl(fd, F_GETFL);
+	if (flags == -1) {
+		TAP_LOG(WARNING,
+			"Unable to get %s current flags\n",
+			ifr.ifr_name);
+		goto error;
+	}
+
 	/* Always set the file descriptor to non-blocking */
-	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
+	flags |= O_NONBLOCK;
+	if (fcntl(fd, F_SETFL, flags) < 0) {
 		TAP_LOG(WARNING,
 			"Unable to set %s to nonblocking: %s",
 			ifr.ifr_name, strerror(errno));
 		goto error;
 	}
 
-	/* Set up trigger to optimize empty Rx bursts */
-	errno = 0;
-	do {
+	/* Find a free realtime signal */
+	for (signo = SIGRTMIN + 1; signo < SIGRTMAX; signo++) {
 		struct sigaction sa;
-		int flags = fcntl(fd, F_GETFL);
 
-		if (flags == -1 || sigaction(SIGIO, NULL, &sa) == -1)
+		if (sigaction(signo, NULL, &sa) == -1) {
+			TAP_LOG(WARNING,
+				"Unable to get current rt-signal %d handler",
+				signo);
+			goto error;
+		}
+
+		/* Already have the handler we want on this signal  */
+		if (sa.sa_handler == tap_trigger_cb)
 			break;
-		if (sa.sa_handler != tap_trigger_cb) {
-			/*
-			 * Make sure SIGIO is not already taken. This is done
-			 * as late as possible to leave the application a
-			 * chance to set up its own signal handler first.
-			 */
-			if (sa.sa_handler != SIG_IGN &&
-			    sa.sa_handler != SIG_DFL) {
-				errno = EBUSY;
-				break;
-			}
-			sa = (struct sigaction){
-				.sa_flags = SA_RESTART,
-				.sa_handler = tap_trigger_cb,
-			};
-			if (sigaction(SIGIO, &sa, NULL) == -1)
-				break;
+
+		/* Is handler in use by application */
+		if (sa.sa_handler != SIG_DFL) {
+			TAP_LOG(DEBUG,
+				"Skipping used rt-signal %d", signo);
+			continue;
 		}
-		/* Enable SIGIO on file descriptor */
-		fcntl(fd, F_SETFL, flags | O_ASYNC);
-		fcntl(fd, F_SETOWN, getpid());
-	} while (0);
 
-	if (errno) {
+		sa = (struct sigaction) {
+			.sa_flags = SA_RESTART,
+			.sa_handler = tap_trigger_cb,
+		};
+
+		if (sigaction(signo, &sa, NULL) == -1) {
+			TAP_LOG(WARNING,
+				"Unable to set rt-signal %d handler\n", signo);
+			goto error;
+		}
+
+		/* Found a good signal to use */
+		TAP_LOG(DEBUG,
+			"Using rt-signal %d", signo);
+		break;
+	}
+
+	if (signo == SIGRTMAX) {
+		TAP_LOG(WARNING, "All rt-signals are in use\n");
+
 		/* Disable trigger globally in case of error */
 		tap_trigger = 0;
-		TAP_LOG(WARNING, "Rx trigger disabled: %s",
-			strerror(errno));
-	}
+		TAP_LOG(NOTICE, "No Rx trigger signal available\n");
+	} else {
+		/* Enable signal on file descriptor */
+		if (fcntl(fd, F_SETSIG, signo) < 0) {
+			TAP_LOG(WARNING, "Unable to set signo %d for fd %d: %s",
+				signo, fd, strerror(errno));
+			goto error;
+		}
+		if (fcntl(fd, F_SETFL, flags | O_ASYNC) < 0) {
+			TAP_LOG(WARNING, "Unable to set fcntl flags: %s",
+				strerror(errno));
+			goto error;
+		}
 
+		if (fcntl(fd, F_SETOWN, getpid()) < 0) {
+			TAP_LOG(WARNING, "Unable to set fcntl owner: %s",
+				strerror(errno));
+			goto error;
+		}
+	}
 	return fd;
 
 error:
-- 
2.27.0


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

* Re: [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-07-14 23:58 ` [dpdk-dev] [PATCH] " Stephen Hemminger
@ 2020-07-27 13:19   ` Morten Brørup
  2020-07-27 19:44     ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Morten Brørup @ 2020-07-27 13:19 UTC (permalink / raw)
  To: Stephen Hemminger, keith.wiles; +Cc: dev

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Wednesday, July 15, 2020 1:58 AM
> 
> SIGIO maybe used by application, instead choose another rt-signal.
> Linux allows any signal to be used for signal based IO.
> Search for an unused signal in the available rt-signal range.

Just an observation. Feel free to ignore at your convenience:

The problem is the same as for SIGIO if the application sets up its own signal handler after this, and uses some hardcoded rt-signal that happens to be the one found to be free.

Unless the application doesn't use a hardcoded rt-signal, but also searches for an unused one.

So perhaps the "search for unused rt-signal" should be exposed as a generic support function for the application (and this driver) to use.

> 
> Add more error checking for fcntl and signal handling.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Resending original version, marked as PATCH now.
> 
>  drivers/net/tap/rte_eth_tap.c | 99 ++++++++++++++++++++++++-----------
>  1 file changed, 67 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c
> b/drivers/net/tap/rte_eth_tap.c
> index 339f24bf82f3..b19e26ba0e65 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -134,7 +134,7 @@ tun_alloc(struct pmd_internals *pmd, int
> is_keepalive)
>  #ifdef IFF_MULTI_QUEUE
>  	unsigned int features;
>  #endif
> -	int fd;
> +	int fd, signo, flags;
> 
>  	memset(&ifr, 0, sizeof(struct ifreq));
> 
> @@ -199,52 +199,87 @@ tun_alloc(struct pmd_internals *pmd, int
> is_keepalive)
>  		}
>  	}
> 
> +	flags = fcntl(fd, F_GETFL);
> +	if (flags == -1) {
> +		TAP_LOG(WARNING,
> +			"Unable to get %s current flags\n",
> +			ifr.ifr_name);
> +		goto error;
> +	}
> +
>  	/* Always set the file descriptor to non-blocking */
> -	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> +	flags |= O_NONBLOCK;
> +	if (fcntl(fd, F_SETFL, flags) < 0) {
>  		TAP_LOG(WARNING,
>  			"Unable to set %s to nonblocking: %s",
>  			ifr.ifr_name, strerror(errno));
>  		goto error;
>  	}
> 
> -	/* Set up trigger to optimize empty Rx bursts */
> -	errno = 0;
> -	do {
> +	/* Find a free realtime signal */
> +	for (signo = SIGRTMIN + 1; signo < SIGRTMAX; signo++) {
>  		struct sigaction sa;
> -		int flags = fcntl(fd, F_GETFL);
> 
> -		if (flags == -1 || sigaction(SIGIO, NULL, &sa) == -1)
> +		if (sigaction(signo, NULL, &sa) == -1) {
> +			TAP_LOG(WARNING,
> +				"Unable to get current rt-signal %d handler",
> +				signo);
> +			goto error;
> +		}
> +
> +		/* Already have the handler we want on this signal  */
> +		if (sa.sa_handler == tap_trigger_cb)
>  			break;
> -		if (sa.sa_handler != tap_trigger_cb) {
> -			/*
> -			 * Make sure SIGIO is not already taken. This is done
> -			 * as late as possible to leave the application a
> -			 * chance to set up its own signal handler first.
> -			 */
> -			if (sa.sa_handler != SIG_IGN &&
> -			    sa.sa_handler != SIG_DFL) {
> -				errno = EBUSY;
> -				break;
> -			}
> -			sa = (struct sigaction){
> -				.sa_flags = SA_RESTART,
> -				.sa_handler = tap_trigger_cb,
> -			};
> -			if (sigaction(SIGIO, &sa, NULL) == -1)
> -				break;
> +
> +		/* Is handler in use by application */
> +		if (sa.sa_handler != SIG_DFL) {
> +			TAP_LOG(DEBUG,
> +				"Skipping used rt-signal %d", signo);
> +			continue;
>  		}
> -		/* Enable SIGIO on file descriptor */
> -		fcntl(fd, F_SETFL, flags | O_ASYNC);
> -		fcntl(fd, F_SETOWN, getpid());
> -	} while (0);
> 
> -	if (errno) {
> +		sa = (struct sigaction) {
> +			.sa_flags = SA_RESTART,
> +			.sa_handler = tap_trigger_cb,
> +		};
> +
> +		if (sigaction(signo, &sa, NULL) == -1) {
> +			TAP_LOG(WARNING,
> +				"Unable to set rt-signal %d handler\n", signo);
> +			goto error;
> +		}
> +
> +		/* Found a good signal to use */
> +		TAP_LOG(DEBUG,
> +			"Using rt-signal %d", signo);
> +		break;
> +	}
> +
> +	if (signo == SIGRTMAX) {
> +		TAP_LOG(WARNING, "All rt-signals are in use\n");
> +
>  		/* Disable trigger globally in case of error */
>  		tap_trigger = 0;
> -		TAP_LOG(WARNING, "Rx trigger disabled: %s",
> -			strerror(errno));
> -	}
> +		TAP_LOG(NOTICE, "No Rx trigger signal available\n");
> +	} else {
> +		/* Enable signal on file descriptor */
> +		if (fcntl(fd, F_SETSIG, signo) < 0) {
> +			TAP_LOG(WARNING, "Unable to set signo %d for fd %d:
> %s",
> +				signo, fd, strerror(errno));
> +			goto error;
> +		}
> +		if (fcntl(fd, F_SETFL, flags | O_ASYNC) < 0) {
> +			TAP_LOG(WARNING, "Unable to set fcntl flags: %s",
> +				strerror(errno));
> +			goto error;
> +		}
> 
> +		if (fcntl(fd, F_SETOWN, getpid()) < 0) {
> +			TAP_LOG(WARNING, "Unable to set fcntl owner: %s",
> +				strerror(errno));
> +			goto error;
> +		}
> +	}
>  	return fd;
> 
>  error:
> --
> 2.27.0
> 


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

* Re: [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-07-27 13:19   ` Morten Brørup
@ 2020-07-27 19:44     ` Stephen Hemminger
  2020-07-28 10:48       ` Morten Brørup
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-07-27 19:44 UTC (permalink / raw)
  To: Morten Brørup; +Cc: keith.wiles, dev

On Mon, 27 Jul 2020 15:19:32 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Wednesday, July 15, 2020 1:58 AM
> > 
> > SIGIO maybe used by application, instead choose another rt-signal.
> > Linux allows any signal to be used for signal based IO.
> > Search for an unused signal in the available rt-signal range.  
> 
> Just an observation. Feel free to ignore at your convenience:
> 
> The problem is the same as for SIGIO if the application sets up its own signal handler after this, and uses some hardcoded rt-signal that happens to be the one found to be free.
> 
> Unless the application doesn't use a hardcoded rt-signal, but also searches for an unused one.
> 
> So perhaps the "search for unused rt-signal" should be exposed as a generic support function for the application (and this driver) to use.

There is no safe way to use a signal deep inside DPDK in a driver.

This is not the kind of thing that should be exposed to the application.

The algorithm for finding an RT signal conforms to the recommended policy on the signal(7)
manual page.

       programs should never refer to real-time signals using hard-
       coded numbers, but instead should always refer to real-time signals
       using the notation SIGRTMIN+n, and include suitable (run-time) checks
       that SIGRTMIN+n does not exceed SIGRTMAX.

The application should be following the proscribed policy on the man page.
If it doesn't it is broken.

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

* Re: [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-07-27 19:44     ` Stephen Hemminger
@ 2020-07-28 10:48       ` Morten Brørup
  2020-08-17 15:26         ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Morten Brørup @ 2020-07-28 10:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: keith.wiles, dev

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Monday, July 27, 2020 9:44 PM
> 
> On Mon, 27 Jul 2020 15:19:32 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> Hemminger
> > > Sent: Wednesday, July 15, 2020 1:58 AM
> > >
> > > SIGIO maybe used by application, instead choose another rt-signal.
> > > Linux allows any signal to be used for signal based IO.
> > > Search for an unused signal in the available rt-signal range.
> >
> > Just an observation. Feel free to ignore at your convenience:
> >
> > The problem is the same as for SIGIO if the application sets up its
> own signal handler after this, and uses some hardcoded rt-signal that
> happens to be the one found to be free.
> >
> > Unless the application doesn't use a hardcoded rt-signal, but also
> searches for an unused one.
> >
> > So perhaps the "search for unused rt-signal" should be exposed as a
> generic support function for the application (and this driver) to use.
> 
> There is no safe way to use a signal deep inside DPDK in a driver.
> 
> This is not the kind of thing that should be exposed to the
> application.
> 
> The algorithm for finding an RT signal conforms to the recommended
> policy on the signal(7)
> manual page.
> 
>        programs should never refer to real-time signals using hard-
>        coded numbers, but instead should always refer to real-time
> signals
>        using the notation SIGRTMIN+n, and include suitable (run-time)
> checks
>        that SIGRTMIN+n does not exceed SIGRTMAX.
> 
> The application should be following the proscribed policy on the man
> page.
> If it doesn't it is broken.

Great. That fully addresses my concern.

Acked-by: Morten Brørup <mb@smartsharesystems.com>


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

* Re: [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-07-28 10:48       ` Morten Brørup
@ 2020-08-17 15:26         ` Ferruh Yigit
  2020-08-25 19:33           ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-08-17 15:26 UTC (permalink / raw)
  To: Morten Brørup, Stephen Hemminger; +Cc: keith.wiles, dev

On 7/28/2020 11:48 AM, Morten Brørup wrote:
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
>> Sent: Monday, July 27, 2020 9:44 PM
>>
>> On Mon, 27 Jul 2020 15:19:32 +0200
>> Morten Brørup <mb@smartsharesystems.com> wrote:
>>
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
>> Hemminger
>>>> Sent: Wednesday, July 15, 2020 1:58 AM
>>>>
>>>> SIGIO maybe used by application, instead choose another rt-signal.
>>>> Linux allows any signal to be used for signal based IO.
>>>> Search for an unused signal in the available rt-signal range.
>>>
>>> Just an observation. Feel free to ignore at your convenience:
>>>
>>> The problem is the same as for SIGIO if the application sets up its
>> own signal handler after this, and uses some hardcoded rt-signal that
>> happens to be the one found to be free.
>>>
>>> Unless the application doesn't use a hardcoded rt-signal, but also
>> searches for an unused one.
>>>
>>> So perhaps the "search for unused rt-signal" should be exposed as a
>> generic support function for the application (and this driver) to use.
>>
>> There is no safe way to use a signal deep inside DPDK in a driver.
>>
>> This is not the kind of thing that should be exposed to the
>> application.
>>
>> The algorithm for finding an RT signal conforms to the recommended
>> policy on the signal(7)
>> manual page.
>>
>>        programs should never refer to real-time signals using hard-
>>        coded numbers, but instead should always refer to real-time
>> signals
>>        using the notation SIGRTMIN+n, and include suitable (run-time)
>> checks
>>        that SIGRTMIN+n does not exceed SIGRTMAX.
>>
>> The application should be following the proscribed policy on the man
>> page.
>> If it doesn't it is broken.
> 
> Great. That fully addresses my concern.
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

* Re: [dpdk-dev] [PATCH] net/tap: avoid using SIGIO
  2020-08-17 15:26         ` Ferruh Yigit
@ 2020-08-25 19:33           ` Ferruh Yigit
  0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-08-25 19:33 UTC (permalink / raw)
  To: Morten Brørup, Stephen Hemminger; +Cc: keith.wiles, dev

On 8/17/2020 4:26 PM, Ferruh Yigit wrote:
> On 7/28/2020 11:48 AM, Morten Brørup wrote:
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
>>> Sent: Monday, July 27, 2020 9:44 PM
>>>
>>> On Mon, 27 Jul 2020 15:19:32 +0200
>>> Morten Brørup <mb@smartsharesystems.com> wrote:
>>>
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
>>> Hemminger
>>>>> Sent: Wednesday, July 15, 2020 1:58 AM
>>>>>
>>>>> SIGIO maybe used by application, instead choose another rt-signal.
>>>>> Linux allows any signal to be used for signal based IO.
>>>>> Search for an unused signal in the available rt-signal range.
>>>>
>>>> Just an observation. Feel free to ignore at your convenience:
>>>>
>>>> The problem is the same as for SIGIO if the application sets up its
>>> own signal handler after this, and uses some hardcoded rt-signal that
>>> happens to be the one found to be free.
>>>>
>>>> Unless the application doesn't use a hardcoded rt-signal, but also
>>> searches for an unused one.
>>>>
>>>> So perhaps the "search for unused rt-signal" should be exposed as a
>>> generic support function for the application (and this driver) to use.
>>>
>>> There is no safe way to use a signal deep inside DPDK in a driver.
>>>
>>> This is not the kind of thing that should be exposed to the
>>> application.
>>>
>>> The algorithm for finding an RT signal conforms to the recommended
>>> policy on the signal(7)
>>> manual page.
>>>
>>>        programs should never refer to real-time signals using hard-
>>>        coded numbers, but instead should always refer to real-time
>>> signals
>>>        using the notation SIGRTMIN+n, and include suitable (run-time)
>>> checks
>>>        that SIGRTMIN+n does not exceed SIGRTMAX.
>>>
>>> The application should be following the proscribed policy on the man
>>> page.
>>> If it doesn't it is broken.
>>
>> Great. That fully addresses my concern.
>>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/main, thanks.

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

end of thread, other threads:[~2020-08-25 19:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-22  1:01 [dpdk-dev] [RFC] net/tap: avoid using SIGIO Stephen Hemminger
2020-07-14 23:58 ` [dpdk-dev] [PATCH] " Stephen Hemminger
2020-07-27 13:19   ` Morten Brørup
2020-07-27 19:44     ` Stephen Hemminger
2020-07-28 10:48       ` Morten Brørup
2020-08-17 15:26         ` Ferruh Yigit
2020-08-25 19:33           ` Ferruh Yigit

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).