Can the PCP field be set in the netdev table?

Can the PCP field be set in the netdev table?

[Kevin Vigouroux]

Hi!

I tried in vain to modify the PCP field with many rules. Nothing worked.

#+begin_src
table netdev t {
	chain in_update_vlan {
		vlan type arp counter
		ip saddr 192.168.60.5 icmp type echo-request counter
	}

	chain in {
		type filter hook ingress device "end0" priority filter; policy accept;
		ether saddr 96:3f:66:77:df:88 vlan id 5 jump in_update_vlan
	}

	chain out_update_vlan {
		vlan type arp meta priority set 0:6 counter
		ip daddr 192.168.60.5 icmp type echo-reply meta priority set 0:6 counter
	}

	chain out {
		type filter hook egress device "end0" priority filter; policy accept;
		ether daddr 96:3f:66:77:df:88 vlan id 5 jump out_update_vlan
	}
}
#+end_src

#+begin_src
table netdev t {
	chain out {
		type filter hook egress device "end0" priority filter; policy accept;
		ether saddr 1f:9b:96:c7:14:d3 vlan pcp 0 vlan pcp set 2 counter
	}
}
#+end_src

#+begin_src
table netdev t {
	chain out {
		type filter hook egress device "end0" priority filter; policy accept;
		ip daddr 192.168.60.5 icmp type echo-reply meta priority set 0:6 meta nftrace set 1
	}
}
#+end_src

#+begin_src
$ sudo nft --debug=netlink add rule netdev t out vlan pcp set 1 counter
netdev filter t
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 2b @ link header + 12 => reg 1 ]
  [ cmp eq reg 1 0x00000081 ]
  [ payload load 1b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x00000020 ]
  [ payload write reg 1 => 1b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
  [ counter pkts 0 bytes 0 ]

$ sudo nft --debug=netlink add rule netdev t out vlan pcp set 6 counter
netdev filter t
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 2b @ link header + 12 => reg 1 ]
  [ cmp eq reg 1 0x00000081 ]
  [ payload load 1b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x000000c0 ]
  [ payload write reg 1 => 1b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
  [ counter pkts 0 bytes 0 ]
#+end_src

I don't have enough knowledge to understand this issue. My device is a 802.1Q
VLAN whose master device is an Ethernet NIC (this is not a switch or software
bridge). Is it a “driver” issue?

Any help?

--
Best regards,
Kevin Vigouroux

Re: Can the PCP field be set in the netdev table?

[Sunny73Cr]

> [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x000000c0 ]
Mask one byte with 31 (lower 5 bits), then XOR with 192 (Upper 2 bits).
Why not just set a value of 192? It isn't what was asked for, either.

I would hazard a guess, and say that it is broken.

It appears as if the EtherType is being re-written to 192; which indicates that it is now an 'old ethernet' length field: definitely broken.

I'll have to look into this issue also; though, I can't provide a fix just yet. Hopefully somebody with more experience can chip in here.

sunny

Re: Can the PCP field be set in the netdev table?

[Sunny73Cr]

I am only assuming here. I made a mistake, and I should make it clear:

Link Layer header would be:

6 bytes destination (MAC) address,
6 bytes source (MAC) address,
4 bytes likely the 802.1Q Priority tag,
2 bytes are likely the ethertype... and the packet continues after...

> [ payload load 2b @ link header + 12 => reg 1 ]
Means match the first half of the 802.1Q header. This is the TPID or Tag Protocol Identifier. Consider the IANA registry for IEEE registered ethernet protocols here: https://iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml
A value of 0x8100 is the 'Customer VLAN' tag; a standard VLAN tag.

> [ cmp eq reg 1 0x00000081 ]
It appears to compare the 'tag space' to '0x81'; though, it may be 'big byte order, little endian bit order' encoding... I am unsure. If all little endian, the match is wrong, if 'big byte, little bit' endian; the match is correct.

> [ payload load 1b @ link header + 14 => reg 1 ]
Now, match the Priority Code Point/Drop Eligible Indicator/Virtual LAN ID.

> [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x000000c0 ]
Considering the input:
> $ sudo nft --debug=netlink add rule netdev t out vlan pcp set 6 counter
Then the result is:
Source (binary LE):    00000110
AND 31 (low 5 bits):   NNNYYYYY -> retain the value of 6
XOR 192 (high 2 bits): 11000110 -> Decimal value of 198

Instead of matching a priority of 6, it is matching a priority of 198.

sunny

Re: Can the PCP field be set in the netdev table?

[Sunny73Cr]

Another correction....

It is matching a priority of 6, DEI of 0, VID of 96.

sunny

Re: Can the PCP field be set in the netdev table?

[Kevin Vigouroux]

Thanks for the explanation!

However, the exclusive OR between bits 1 and 0 results in 0. Consequently, only
the PCP field is modified by the operation.

--
Best regards,
Kevin Vigouroux

Re: Can the PCP field be set in the netdev table?

[Florian Westphal]

Kevin Vigouroux <ke.vigouroux@laposte.net> wrote:
> Hi!
> 
> I tried in vain to modify the PCP field with many rules. Nothing worked.

Yes, this is broken, likely due to vlan hw tag offload.

>   [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x00000020 ]
>   [ payload write reg 1 => 1b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
                             ~~

kernel doesn't handle 1 byte writes to the vlan pseudoheader at this
time, so this will need a userspace/nft fixup to expand this to a 2 byte write.

I can look at it next week (need to create a vlan test setup first).

You could try to disable HW offload of the vlan tag and see if that helps.

with patch, this will emit a 2byte load/store which kernel should be
able to handle:

  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 2b @ link header + 12 => reg 1 ]
  [ cmp eq reg 1 0x00000081 ]
  [ payload load 2b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000ff1f ) ^ 0x00000020 ]
  [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
  [ counter pkts 0 bytes 0 ]

A short in the dark, compile tested only, no idea if this is enough to
make it work.

diff --git a/src/evaluate.c b/src/evaluate.c
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3258,6 +3258,24 @@ static bool stmt_evaluate_payload_need_csum(const struct expr *payload)
 	return desc && desc->checksum_key;
 }
 
+static bool stmt_evaluate_is_vlan(const struct expr *payload)
+{
+	return payload->payload.base == PROTO_BASE_LL_HDR &&
+	       payload->payload.desc == &proto_vlan;
+}
+
+static bool stmt_evaluate_payload_need_aligned_fetch(const struct expr *payload)
+{
+
+	if (stmt_evaluate_payload_need_csum(payload))
+		return true;
+
+	if (stmt_evaluate_is_vlan(payload))
+		return true;
+
+	return false;
+}
+
 static int stmt_evaluate_exthdr(struct eval_ctx *ctx, struct stmt *stmt)
 {
 	struct expr *exthdr;
@@ -3287,7 +3305,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 	unsigned int masklen, extra_len = 0;
 	struct expr *payload;
 	mpz_t bitmask, ff;
-	bool need_csum;
+	bool aligned_fetch;
 
 	if (stmt->payload.expr->payload.inner_desc) {
 		return expr_error(ctx->msgs, stmt->payload.expr,
@@ -3310,7 +3328,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 	if (stmt->payload.val->etype == EXPR_RANGE)
 		return stmt_error_range(ctx, stmt, stmt->payload.val);
 
-	need_csum = stmt_evaluate_payload_need_csum(payload);
+	aligned_fetch = stmt_evaluate_payload_need_aligned_fetch(payload);
 
 	if (!payload_needs_adjustment(payload)) {
 
@@ -3318,7 +3336,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 		 * update checksum and the length is not even because
 		 * kernel checksum functions cannot deal with odd lengths.
 		 */
-		if (!need_csum || ((payload->len / BITS_PER_BYTE) & 1) == 0)
+		if (!aligned_fetch || ((payload->len / BITS_PER_BYTE) & 1) == 0)
 			return 0;
 	}
 
@@ -3334,7 +3352,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 				  "uneven load cannot span more than %u bytes, got %u",
 				  sizeof(data), payload_byte_size);
 
-	if (need_csum && payload_byte_size & 1) {
+	if (aligned_fetch && payload_byte_size & 1) {
 		payload_byte_size++;
 
 		if (payload_byte_offset & 1) { /* prefer 16bit aligned fetch */

Re: Can the PCP field be set in the netdev table?

[Kevin Vigouroux]

Hi!

I tested the patch and it seems to work. Disabling HW offload does not seem to
work. I performed my test by sending ICMP packets between two stations connected
by a switch on a dedicated VLAN. I noticed that tcpdump does not detect that the
PCP field is modified on the sending station. On the other hand, the network
capture on the receiving station shows that the PCP field is modified.

--
Best regards,
Kevin Vigouroux