2022-01-28 08:12:12

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 net-next 0/4] ice: switch: debloat packet templates code

This hunts down several places around packet templates/dummies for
switch rules which are either repetitive, fragile or just not
really readable code.
It's a common need to add new packet templates and to review such
changes as well, try to simplify both with the help of a pair
macros and aliases.

bloat-o-meter is happy about that (built w/ LLVM 13):

add/remove: 0/1 grow/shrink: 2/0 up/down: 148/-202 (-54)
Function old new delta
ice_add_adv_rule 2383 2529 +146
ice_fill_adv_dummy_packet 289 291 +2
ice_adv_add_update_vsi_list 202 - -202
Total: Before=395813, After=395759, chg -0.01%

Diffstat also looks nice, and adding new packet templates now takes
less lines.

We'll probably come out with dynamic template crafting in a while,
but for now let's improve what we have currently.

Note: this will conflict with [1] going through net-next,
a followup will be sent once accepted.

From v1 ([0]):
- rebase on top of the latest next-queue (to fix #3 not applying);
- adjust the kdoc accordingly to the function proto changes in #3;
- no functional changes.

[0] https://lore.kernel.org/netdev/[email protected]
[1] https://lore.kernel.org/netdev/[email protected]

Alexander Lobakin (4):
ice: switch: add and use u16[] aliases to ice_adv_lkup_elem::{h,m}_u
ice: switch: unobscurify bitops loop in ice_fill_adv_dummy_packet()
ice: switch: use a struct to pass packet template params
ice: switch: use convenience macros to declare dummy pkt templates

drivers/net/ethernet/intel/ice/ice_switch.c | 273 ++++++++------------
drivers/net/ethernet/intel/ice/ice_switch.h | 12 +-
2 files changed, 123 insertions(+), 162 deletions(-)

--
2.34.1


2022-01-28 08:36:47

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 net-next 2/4] ice: switch: unobscurify bitops loop in ice_fill_adv_dummy_packet()

A loop performing header modification according to the provided mask
in ice_fill_adv_dummy_packet() is very cryptic (and error-prone).
Replace two identical cast-deferences with a variable. Replace three
struct-member-array-accesses with a variable. Invert the condition,
reduce the indentation by one -> eliminate line wraps.

Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_switch.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 7f7e929e73a8..b6b6e8f5d358 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -5266,13 +5266,15 @@ ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
* indicated by the mask to make sure we don't improperly write
* over any significant packet data.
*/
- for (j = 0; j < len / sizeof(u16); j++)
- if (lkups[i].m_raw[j])
- ((u16 *)(pkt + offset))[j] =
- (((u16 *)(pkt + offset))[j] &
- ~lkups[i].m_raw[j]) |
- (lkups[i].h_raw[j] &
- lkups[i].m_raw[j]);
+ for (j = 0; j < len / sizeof(u16); j++) {
+ u16 *ptr = (u16 *)(pkt + offset);
+ u16 mask = lkups[i].m_raw[j];
+
+ if (!mask)
+ continue;
+
+ ptr[j] = (ptr[j] & ~mask) | (lkups[i].h_raw[j] & mask);
+ }
}

s_rule->pdata.lkup_tx_rx.hdr_len = cpu_to_le16(pkt_len);
--
2.34.1

2022-01-28 08:36:47

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 net-next 4/4] ice: switch: use convenience macros to declare dummy pkt templates

Declarations of dummy/template packet headers and offsets can be
minified to improve readability and simplify adding new templates.
Move all the repetitive constructions into two macros and let them
do the name and type expansions.
Linewrap removal is yet another positive side effect.

Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_switch.c | 83 +++++++++++----------
1 file changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 68202671a114..2105cae0f2f8 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -41,15 +41,22 @@ struct ice_dummy_pkt_profile {
u16 pkt_len;
};

+#define ICE_PKT_OFFSETS(type) \
+ static const struct ice_dummy_pkt_offsets \
+ ice_dummy_##type##_packet_offsets[]
+
+#define ICE_PKT_TEMPLATE(type) \
+ static const u8 ice_dummy_##type##_packet[]
+
#define ICE_PKT_PROFILE(type) ({ \
(struct ice_dummy_pkt_profile){ \
- .pkt = dummy_##type##_packet, \
- .pkt_len = sizeof(dummy_##type##_packet), \
- .offsets = dummy_##type##_packet_offsets, \
+ .pkt = ice_dummy_##type##_packet, \
+ .pkt_len = sizeof(ice_dummy_##type##_packet), \
+ .offsets = ice_dummy_##type##_packet_offsets, \
}; \
})

-static const struct ice_dummy_pkt_offsets dummy_gre_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(gre_tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -61,7 +68,7 @@ static const struct ice_dummy_pkt_offsets dummy_gre_tcp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_gre_tcp_packet[] = {
+ICE_PKT_TEMPLATE(gre_tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -96,7 +103,7 @@ static const u8 dummy_gre_tcp_packet[] = {
0x00, 0x00, 0x00, 0x00
};

-static const struct ice_dummy_pkt_offsets dummy_gre_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(gre_udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -108,7 +115,7 @@ static const struct ice_dummy_pkt_offsets dummy_gre_udp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_gre_udp_packet[] = {
+ICE_PKT_TEMPLATE(gre_udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -140,7 +147,7 @@ static const u8 dummy_gre_udp_packet[] = {
0x00, 0x08, 0x00, 0x00,
};

-static const struct ice_dummy_pkt_offsets dummy_udp_tun_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp_tun_tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -155,7 +162,7 @@ static const struct ice_dummy_pkt_offsets dummy_udp_tun_tcp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_udp_tun_tcp_packet[] = {
+ICE_PKT_TEMPLATE(udp_tun_tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -193,7 +200,7 @@ static const u8 dummy_udp_tun_tcp_packet[] = {
0x00, 0x00, 0x00, 0x00
};

-static const struct ice_dummy_pkt_offsets dummy_udp_tun_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp_tun_udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -208,7 +215,7 @@ static const struct ice_dummy_pkt_offsets dummy_udp_tun_udp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_udp_tun_udp_packet[] = {
+ICE_PKT_TEMPLATE(udp_tun_udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -243,8 +250,7 @@ static const u8 dummy_udp_tun_udp_packet[] = {
0x00, 0x08, 0x00, 0x00,
};

-static const struct ice_dummy_pkt_offsets
-dummy_gre_ipv6_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(gre_ipv6_tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -256,7 +262,7 @@ dummy_gre_ipv6_tcp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_gre_ipv6_tcp_packet[] = {
+ICE_PKT_TEMPLATE(gre_ipv6_tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -296,8 +302,7 @@ static const u8 dummy_gre_ipv6_tcp_packet[] = {
0x00, 0x00, 0x00, 0x00
};

-static const struct ice_dummy_pkt_offsets
-dummy_gre_ipv6_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(gre_ipv6_udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -309,7 +314,7 @@ dummy_gre_ipv6_udp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_gre_ipv6_udp_packet[] = {
+ICE_PKT_TEMPLATE(gre_ipv6_udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -346,8 +351,7 @@ static const u8 dummy_gre_ipv6_udp_packet[] = {
0x00, 0x08, 0x00, 0x00,
};

-static const struct ice_dummy_pkt_offsets
-dummy_udp_tun_ipv6_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp_tun_ipv6_tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -362,7 +366,7 @@ dummy_udp_tun_ipv6_tcp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_udp_tun_ipv6_tcp_packet[] = {
+ICE_PKT_TEMPLATE(udp_tun_ipv6_tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -405,8 +409,7 @@ static const u8 dummy_udp_tun_ipv6_tcp_packet[] = {
0x00, 0x00, 0x00, 0x00
};

-static const struct ice_dummy_pkt_offsets
-dummy_udp_tun_ipv6_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp_tun_ipv6_udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -421,7 +424,7 @@ dummy_udp_tun_ipv6_udp_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_udp_tun_ipv6_udp_packet[] = {
+ICE_PKT_TEMPLATE(udp_tun_ipv6_udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -462,7 +465,7 @@ static const u8 dummy_udp_tun_ipv6_udp_packet[] = {
};

/* offset info for MAC + IPv4 + UDP dummy packet */
-static const struct ice_dummy_pkt_offsets dummy_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -471,7 +474,7 @@ static const struct ice_dummy_pkt_offsets dummy_udp_packet_offsets[] = {
};

/* Dummy packet for MAC + IPv4 + UDP */
-static const u8 dummy_udp_packet[] = {
+ICE_PKT_TEMPLATE(udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -491,7 +494,7 @@ static const u8 dummy_udp_packet[] = {
};

/* offset info for MAC + VLAN + IPv4 + UDP dummy packet */
-static const struct ice_dummy_pkt_offsets dummy_vlan_udp_packet_offsets[] = {
+ICE_PKT_OFFSETS(vlan_udp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_VLAN_OFOS, 12 },
{ ICE_ETYPE_OL, 16 },
@@ -501,7 +504,7 @@ static const struct ice_dummy_pkt_offsets dummy_vlan_udp_packet_offsets[] = {
};

/* C-tag (801.1Q), IPv4:UDP dummy packet */
-static const u8 dummy_vlan_udp_packet[] = {
+ICE_PKT_TEMPLATE(vlan_udp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -523,7 +526,7 @@ static const u8 dummy_vlan_udp_packet[] = {
};

/* offset info for MAC + IPv4 + TCP dummy packet */
-static const struct ice_dummy_pkt_offsets dummy_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV4_OFOS, 14 },
@@ -532,7 +535,7 @@ static const struct ice_dummy_pkt_offsets dummy_tcp_packet_offsets[] = {
};

/* Dummy packet for MAC + IPv4 + TCP */
-static const u8 dummy_tcp_packet[] = {
+ICE_PKT_TEMPLATE(tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -555,7 +558,7 @@ static const u8 dummy_tcp_packet[] = {
};

/* offset info for MAC + VLAN (C-tag, 802.1Q) + IPv4 + TCP dummy packet */
-static const struct ice_dummy_pkt_offsets dummy_vlan_tcp_packet_offsets[] = {
+ICE_PKT_OFFSETS(vlan_tcp) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_VLAN_OFOS, 12 },
{ ICE_ETYPE_OL, 16 },
@@ -565,7 +568,7 @@ static const struct ice_dummy_pkt_offsets dummy_vlan_tcp_packet_offsets[] = {
};

/* C-tag (801.1Q), IPv4:TCP dummy packet */
-static const u8 dummy_vlan_tcp_packet[] = {
+ICE_PKT_TEMPLATE(vlan_tcp) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -589,7 +592,7 @@ static const u8 dummy_vlan_tcp_packet[] = {
0x00, 0x00, /* 2 bytes for 4 byte alignment */
};

-static const struct ice_dummy_pkt_offsets dummy_tcp_ipv6_packet_offsets[] = {
+ICE_PKT_OFFSETS(tcp_ipv6) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV6_OFOS, 14 },
@@ -597,7 +600,7 @@ static const struct ice_dummy_pkt_offsets dummy_tcp_ipv6_packet_offsets[] = {
{ ICE_PROTOCOL_LAST, 0 },
};

-static const u8 dummy_tcp_ipv6_packet[] = {
+ICE_PKT_TEMPLATE(tcp_ipv6) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -625,8 +628,7 @@ static const u8 dummy_tcp_ipv6_packet[] = {
};

/* C-tag (802.1Q): IPv6 + TCP */
-static const struct ice_dummy_pkt_offsets
-dummy_vlan_tcp_ipv6_packet_offsets[] = {
+ICE_PKT_OFFSETS(vlan_tcp_ipv6) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_VLAN_OFOS, 12 },
{ ICE_ETYPE_OL, 16 },
@@ -636,7 +638,7 @@ dummy_vlan_tcp_ipv6_packet_offsets[] = {
};

/* C-tag (802.1Q), IPv6 + TCP dummy packet */
-static const u8 dummy_vlan_tcp_ipv6_packet[] = {
+ICE_PKT_TEMPLATE(vlan_tcp_ipv6) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -666,7 +668,7 @@ static const u8 dummy_vlan_tcp_ipv6_packet[] = {
};

/* IPv6 + UDP */
-static const struct ice_dummy_pkt_offsets dummy_udp_ipv6_packet_offsets[] = {
+ICE_PKT_OFFSETS(udp_ipv6) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
{ ICE_IPV6_OFOS, 14 },
@@ -675,7 +677,7 @@ static const struct ice_dummy_pkt_offsets dummy_udp_ipv6_packet_offsets[] = {
};

/* IPv6 + UDP dummy packet */
-static const u8 dummy_udp_ipv6_packet[] = {
+ICE_PKT_TEMPLATE(udp_ipv6) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
@@ -703,8 +705,7 @@ static const u8 dummy_udp_ipv6_packet[] = {
};

/* C-tag (802.1Q): IPv6 + UDP */
-static const struct ice_dummy_pkt_offsets
-dummy_vlan_udp_ipv6_packet_offsets[] = {
+ICE_PKT_OFFSETS(vlan_udp_ipv6) = {
{ ICE_MAC_OFOS, 0 },
{ ICE_VLAN_OFOS, 12 },
{ ICE_ETYPE_OL, 16 },
@@ -714,7 +715,7 @@ dummy_vlan_udp_ipv6_packet_offsets[] = {
};

/* C-tag (802.1Q), IPv6 + UDP dummy packet */
-static const u8 dummy_vlan_udp_ipv6_packet[] = {
+ICE_PKT_TEMPLATE(vlan_udp_ipv6) = {
0x00, 0x00, 0x00, 0x00, /* ICE_MAC_OFOS 0 */
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
--
2.34.1

2022-01-28 08:36:47

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 net-next 3/4] ice: switch: use a struct to pass packet template params

ice_find_dummy_packet() contains a lot of boilerplate code and a
nice room for copy-paste mistakes.
Instead of passing 3 separate pointers back and forth to get packet
template (dummy) params, directly return a structure containing
them. Then, use a macro to compose compound literals and avoid code
duplication on return path.
Now, dummy packet type/name is needed only once to return a full
correct triple pkt-pkt_len-offsets, and those are all one-liners.

Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_switch.c | 173 +++++++-------------
1 file changed, 62 insertions(+), 111 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index b6b6e8f5d358..68202671a114 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -35,6 +35,20 @@ struct ice_dummy_pkt_offsets {
u16 offset; /* ICE_PROTOCOL_LAST indicates end of list */
};

+struct ice_dummy_pkt_profile {
+ const struct ice_dummy_pkt_offsets *offsets;
+ const u8 *pkt;
+ u16 pkt_len;
+};
+
+#define ICE_PKT_PROFILE(type) ({ \
+ (struct ice_dummy_pkt_profile){ \
+ .pkt = dummy_##type##_packet, \
+ .pkt_len = sizeof(dummy_##type##_packet), \
+ .offsets = dummy_##type##_packet_offsets, \
+ }; \
+})
+
static const struct ice_dummy_pkt_offsets dummy_gre_tcp_packet_offsets[] = {
{ ICE_MAC_OFOS, 0 },
{ ICE_ETYPE_OL, 12 },
@@ -5035,15 +5049,12 @@ ice_add_adv_recipe(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
* structure per protocol header
* @lkups_cnt: number of protocols
* @tun_type: tunnel type
- * @pkt: dummy packet to fill according to filter match criteria
- * @pkt_len: packet length of dummy packet
- * @offsets: pointer to receive the pointer to the offsets for the packet
+ *
+ * Returns the &ice_dummy_pkt_profile corresponding to these lookup params.
*/
-static void
+static struct ice_dummy_pkt_profile
ice_find_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
- enum ice_sw_tunnel_type tun_type,
- const u8 **pkt, u16 *pkt_len,
- const struct ice_dummy_pkt_offsets **offsets)
+ enum ice_sw_tunnel_type tun_type)
{
bool tcp = false, udp = false, ipv6 = false, vlan = false;
bool ipv6_il = false;
@@ -5073,100 +5084,49 @@ ice_find_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
}

if (tun_type == ICE_SW_TUN_NVGRE) {
- if (tcp && ipv6_il) {
- *pkt = dummy_gre_ipv6_tcp_packet;
- *pkt_len = sizeof(dummy_gre_ipv6_tcp_packet);
- *offsets = dummy_gre_ipv6_tcp_packet_offsets;
- return;
- }
- if (tcp) {
- *pkt = dummy_gre_tcp_packet;
- *pkt_len = sizeof(dummy_gre_tcp_packet);
- *offsets = dummy_gre_tcp_packet_offsets;
- return;
- }
- if (ipv6_il) {
- *pkt = dummy_gre_ipv6_udp_packet;
- *pkt_len = sizeof(dummy_gre_ipv6_udp_packet);
- *offsets = dummy_gre_ipv6_udp_packet_offsets;
- return;
- }
- *pkt = dummy_gre_udp_packet;
- *pkt_len = sizeof(dummy_gre_udp_packet);
- *offsets = dummy_gre_udp_packet_offsets;
- return;
+ if (tcp && ipv6_il)
+ return ICE_PKT_PROFILE(gre_ipv6_tcp);
+ else if (tcp)
+ return ICE_PKT_PROFILE(gre_tcp);
+ else if (ipv6_il)
+ return ICE_PKT_PROFILE(gre_ipv6_udp);
+ else
+ return ICE_PKT_PROFILE(gre_udp);
}

if (tun_type == ICE_SW_TUN_VXLAN ||
tun_type == ICE_SW_TUN_GENEVE) {
- if (tcp && ipv6_il) {
- *pkt = dummy_udp_tun_ipv6_tcp_packet;
- *pkt_len = sizeof(dummy_udp_tun_ipv6_tcp_packet);
- *offsets = dummy_udp_tun_ipv6_tcp_packet_offsets;
- return;
- }
- if (tcp) {
- *pkt = dummy_udp_tun_tcp_packet;
- *pkt_len = sizeof(dummy_udp_tun_tcp_packet);
- *offsets = dummy_udp_tun_tcp_packet_offsets;
- return;
- }
- if (ipv6_il) {
- *pkt = dummy_udp_tun_ipv6_udp_packet;
- *pkt_len = sizeof(dummy_udp_tun_ipv6_udp_packet);
- *offsets = dummy_udp_tun_ipv6_udp_packet_offsets;
- return;
- }
- *pkt = dummy_udp_tun_udp_packet;
- *pkt_len = sizeof(dummy_udp_tun_udp_packet);
- *offsets = dummy_udp_tun_udp_packet_offsets;
- return;
+ if (tcp && ipv6_il)
+ return ICE_PKT_PROFILE(udp_tun_ipv6_tcp);
+ else if (tcp)
+ return ICE_PKT_PROFILE(udp_tun_tcp);
+ else if (ipv6_il)
+ return ICE_PKT_PROFILE(udp_tun_ipv6_udp);
+ else
+ return ICE_PKT_PROFILE(udp_tun_udp);
}

if (udp && !ipv6) {
- if (vlan) {
- *pkt = dummy_vlan_udp_packet;
- *pkt_len = sizeof(dummy_vlan_udp_packet);
- *offsets = dummy_vlan_udp_packet_offsets;
- return;
- }
- *pkt = dummy_udp_packet;
- *pkt_len = sizeof(dummy_udp_packet);
- *offsets = dummy_udp_packet_offsets;
- return;
+ if (vlan)
+ return ICE_PKT_PROFILE(vlan_udp);
+ else
+ return ICE_PKT_PROFILE(udp);
} else if (udp && ipv6) {
- if (vlan) {
- *pkt = dummy_vlan_udp_ipv6_packet;
- *pkt_len = sizeof(dummy_vlan_udp_ipv6_packet);
- *offsets = dummy_vlan_udp_ipv6_packet_offsets;
- return;
- }
- *pkt = dummy_udp_ipv6_packet;
- *pkt_len = sizeof(dummy_udp_ipv6_packet);
- *offsets = dummy_udp_ipv6_packet_offsets;
- return;
+ if (vlan)
+ return ICE_PKT_PROFILE(vlan_udp_ipv6);
+ else
+ return ICE_PKT_PROFILE(udp_ipv6);
} else if ((tcp && ipv6) || ipv6) {
- if (vlan) {
- *pkt = dummy_vlan_tcp_ipv6_packet;
- *pkt_len = sizeof(dummy_vlan_tcp_ipv6_packet);
- *offsets = dummy_vlan_tcp_ipv6_packet_offsets;
- return;
- }
- *pkt = dummy_tcp_ipv6_packet;
- *pkt_len = sizeof(dummy_tcp_ipv6_packet);
- *offsets = dummy_tcp_ipv6_packet_offsets;
- return;
+ if (vlan)
+ return ICE_PKT_PROFILE(vlan_tcp_ipv6);
+ else
+ return ICE_PKT_PROFILE(tcp_ipv6);
}

- if (vlan) {
- *pkt = dummy_vlan_tcp_packet;
- *pkt_len = sizeof(dummy_vlan_tcp_packet);
- *offsets = dummy_vlan_tcp_packet_offsets;
- } else {
- *pkt = dummy_tcp_packet;
- *pkt_len = sizeof(dummy_tcp_packet);
- *offsets = dummy_tcp_packet_offsets;
- }
+ if (vlan)
+ return ICE_PKT_PROFILE(vlan_tcp);
+
+ return ICE_PKT_PROFILE(tcp);
}

/**
@@ -5176,15 +5136,12 @@ ice_find_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
* structure per protocol header
* @lkups_cnt: number of protocols
* @s_rule: stores rule information from the match criteria
- * @dummy_pkt: dummy packet to fill according to filter match criteria
- * @pkt_len: packet length of dummy packet
- * @offsets: offset info for the dummy packet
+ * @profile: dummy packet profile (the template, its size and header offsets)
*/
static int
ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
struct ice_aqc_sw_rules_elem *s_rule,
- const u8 *dummy_pkt, u16 pkt_len,
- const struct ice_dummy_pkt_offsets *offsets)
+ const struct ice_dummy_pkt_profile *profile)
{
u8 *pkt;
u16 i;
@@ -5194,9 +5151,10 @@ ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
*/
pkt = s_rule->pdata.lkup_tx_rx.hdr;

- memcpy(pkt, dummy_pkt, pkt_len);
+ memcpy(pkt, profile->pkt, profile->pkt_len);

for (i = 0; i < lkups_cnt; i++) {
+ const struct ice_dummy_pkt_offsets *offsets = profile->offsets;
enum ice_protocol_type type;
u16 offset = 0, len = 0, j;
bool found = false;
@@ -5277,7 +5235,7 @@ ice_fill_adv_dummy_packet(struct ice_adv_lkup_elem *lkups, u16 lkups_cnt,
}
}

- s_rule->pdata.lkup_tx_rx.hdr_len = cpu_to_le16(pkt_len);
+ s_rule->pdata.lkup_tx_rx.hdr_len = cpu_to_le16(profile->pkt_len);

return 0;
}
@@ -5500,12 +5458,11 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
struct ice_rule_query_data *added_entry)
{
struct ice_adv_fltr_mgmt_list_entry *m_entry, *adv_fltr = NULL;
- u16 rid = 0, i, pkt_len, rule_buf_sz, vsi_handle;
- const struct ice_dummy_pkt_offsets *pkt_offsets;
struct ice_aqc_sw_rules_elem *s_rule = NULL;
+ u16 rid = 0, i, rule_buf_sz, vsi_handle;
+ struct ice_dummy_pkt_profile profile;
struct list_head *rule_head;
struct ice_switch_info *sw;
- const u8 *pkt = NULL;
u16 word_cnt;
u32 act = 0;
int status;
@@ -5533,13 +5490,8 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
if (!word_cnt || word_cnt > ICE_MAX_CHAIN_WORDS)
return -EINVAL;

- /* make sure that we can locate a dummy packet */
- ice_find_dummy_packet(lkups, lkups_cnt, rinfo->tun_type, &pkt, &pkt_len,
- &pkt_offsets);
- if (!pkt) {
- status = -EINVAL;
- goto err_ice_add_adv_rule;
- }
+ /* locate a dummy packet */
+ profile = ice_find_dummy_packet(lkups, lkups_cnt, rinfo->tun_type);

if (!(rinfo->sw_act.fltr_act == ICE_FWD_TO_VSI ||
rinfo->sw_act.fltr_act == ICE_FWD_TO_Q ||
@@ -5580,7 +5532,7 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
}
return status;
}
- rule_buf_sz = ICE_SW_RULE_RX_TX_NO_HDR_SIZE + pkt_len;
+ rule_buf_sz = ICE_SW_RULE_RX_TX_NO_HDR_SIZE + profile.pkt_len;
s_rule = kzalloc(rule_buf_sz, GFP_KERNEL);
if (!s_rule)
return -ENOMEM;
@@ -5640,8 +5592,7 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
s_rule->pdata.lkup_tx_rx.recipe_id = cpu_to_le16(rid);
s_rule->pdata.lkup_tx_rx.act = cpu_to_le32(act);

- status = ice_fill_adv_dummy_packet(lkups, lkups_cnt, s_rule, pkt,
- pkt_len, pkt_offsets);
+ status = ice_fill_adv_dummy_packet(lkups, lkups_cnt, s_rule, &profile);
if (status)
goto err_ice_add_adv_rule;

@@ -5649,7 +5600,7 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
rinfo->tun_type != ICE_SW_TUN_AND_NON_TUN) {
status = ice_fill_adv_packet_tun(hw, rinfo->tun_type,
s_rule->pdata.lkup_tx_rx.hdr,
- pkt_offsets);
+ profile.offsets);
if (status)
goto err_ice_add_adv_rule;
}
--
2.34.1

2022-02-18 05:35:18

by Penigalapati, Sandeep

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v2 net-next 4/4] ice: switch: use convenience macros to declare dummy pkt templates

>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Thursday, January 27, 2022 9:10 PM
>To: [email protected]
>Cc: Szapar-Mudlaw, Martyna <[email protected]>;
>[email protected]; [email protected]; Jakub Kicinski
><[email protected]>; David S. Miller <[email protected]>
>Subject: [Intel-wired-lan] [PATCH v2 net-next 4/4] ice: switch: use convenience
>macros to declare dummy pkt templates
>
>Declarations of dummy/template packet headers and offsets can be minified
>to improve readability and simplify adding new templates.
>Move all the repetitive constructions into two macros and let them do the
>name and type expansions.
>Linewrap removal is yet another positive side effect.
>
>Signed-off-by: Alexander Lobakin <[email protected]>
>---
> drivers/net/ethernet/intel/ice/ice_switch.c | 83 +++++++++++----------
> 1 file changed, 42 insertions(+), 41 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>

2022-02-18 05:38:55

by Penigalapati, Sandeep

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v2 net-next 2/4] ice: switch: unobscurify bitops loop in ice_fill_adv_dummy_packet()

>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Thursday, January 27, 2022 9:10 PM
>To: [email protected]
>Cc: Szapar-Mudlaw, Martyna <[email protected]>;
>[email protected]; [email protected]; Jakub Kicinski
><[email protected]>; David S. Miller <[email protected]>
>Subject: [Intel-wired-lan] [PATCH v2 net-next 2/4] ice: switch: unobscurify
>bitops loop in ice_fill_adv_dummy_packet()
>
>A loop performing header modification according to the provided mask in
>ice_fill_adv_dummy_packet() is very cryptic (and error-prone).
>Replace two identical cast-deferences with a variable. Replace three struct-
>member-array-accesses with a variable. Invert the condition, reduce the
>indentation by one -> eliminate line wraps.
>
>Signed-off-by: Alexander Lobakin <[email protected]>
>---
> drivers/net/ethernet/intel/ice/ice_switch.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>

2022-02-18 06:29:22

by Penigalapati, Sandeep

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v2 net-next 3/4] ice: switch: use a struct to pass packet template params

>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Thursday, January 27, 2022 9:10 PM
>To: [email protected]
>Cc: Szapar-Mudlaw, Martyna <[email protected]>;
>[email protected]; [email protected]; Jakub Kicinski
><[email protected]>; David S. Miller <[email protected]>
>Subject: [Intel-wired-lan] [PATCH v2 net-next 3/4] ice: switch: use a struct to
>pass packet template params
>
>ice_find_dummy_packet() contains a lot of boilerplate code and a nice room
>for copy-paste mistakes.
>Instead of passing 3 separate pointers back and forth to get packet template
>(dummy) params, directly return a structure containing them. Then, use a
>macro to compose compound literals and avoid code duplication on return
>path.
>Now, dummy packet type/name is needed only once to return a full correct
>triple pkt-pkt_len-offsets, and those are all one-liners.
>
>Signed-off-by: Alexander Lobakin <[email protected]>
>---
> drivers/net/ethernet/intel/ice/ice_switch.c | 173 +++++++-------------
> 1 file changed, 62 insertions(+), 111 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>