[net-next,v4,11/12] igc: Check incompatible configs for Frame Preemption

Commit Message

Vinicius Costa Gomes June 26, 2021, 12:33 a.m. UTC

Frame Preemption and LaunchTime cannot be enabled on the same queue.
If that situation happens, emit an error to the user, and log the
error.

Signed-off-by: Vinicius Costa Gomes <[e-mail]>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Vladimir Oltean June 28, 2021, 9:20 a.m. UTC | #1

On Fri, Jun 25, 2021 at 05:33:13PM -0700, Vinicius Costa Gomes wrote:
> Frame Preemption and LaunchTime cannot be enabled on the same queue.
> If that situation happens, emit an error to the user, and log the
> error.
> 
> Signed-off-by: Vinicius Costa Gomes <[e-mail]>
> ---

This is a very interesting limitation, considering the fact that much of
the frame preemption validation that I did was in conjunction with
tc-etf and SO_TXTIME (send packets on 2 queues, one preemptible and one
express, and compare the TX timestamps of the express packets with their
scheduled TX times). The base-time offset between the ET and the PT
packets is varied in small increments in the order of 20 ns or so.
If this is not possible with hardware driven by igc, how do you know it
works properly? :)

Vinicius Costa Gomes April 11, 2022, 11:36 p.m. UTC | #2

Vladimir Oltean <[e-mail]> writes:

> On Fri, Jun 25, 2021 at 05:33:13PM -0700, Vinicius Costa Gomes wrote:
>> Frame Preemption and LaunchTime cannot be enabled on the same queue.
>> If that situation happens, emit an error to the user, and log the
>> error.
>> 
>> Signed-off-by: Vinicius Costa Gomes <[e-mail]>
>> ---
>
> This is a very interesting limitation, considering the fact that much of
> the frame preemption validation that I did was in conjunction with
> tc-etf and SO_TXTIME (send packets on 2 queues, one preemptible and one
> express, and compare the TX timestamps of the express packets with their
> scheduled TX times). The base-time offset between the ET and the PT
> packets is varied in small increments in the order of 20 ns or so.
> If this is not possible with hardware driven by igc, how do you know it
> works properly? :)

Good question. My tests were much less accurate than what you were
doing, I was basically flooding the link with preemptable packets, and
sending some number of express packets, and counting them using some
debug counters on the receiving side.


Cheers,

Patch

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 038383519b10..20dac04a02f2 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5432,6 +5432,11 @@  static int igc_save_launchtime_params(struct igc_adapter *adapter, int queue,
 	if (queue < 0 || queue >= adapter->num_tx_queues)
 		return -EINVAL;
 
+	if (ring->preemptible) {
+		netdev_err(adapter->netdev, "Cannot enable LaunchTime on a preemptible queue\n");
+		return -EINVAL;
+	}
+
 	ring = adapter->tx_ring[queue];
 	ring->launchtime_enable = enable;
 
@@ -5573,8 +5578,14 @@  static int igc_save_frame_preemption(struct igc_adapter *adapter,
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
 		struct igc_ring *ring = adapter->tx_ring[i];
+		bool preemptible = preempt & BIT(i);
 
-		ring->preemptible = preempt & BIT(i);
+		if (ring->launchtime_enable && preemptible) {
+			netdev_err(adapter->netdev, "Cannot set queue as preemptible if LaunchTime is enabled\n");
+			return -EINVAL;
+		}
+
+		ring->preemptible = preemptible;
 	}
 
 	return 0;