[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z7Otj0v0PSKy8nSH@lore-desk>
Date: Mon, 17 Feb 2025 22:43:43 +0100
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Simon Horman <horms@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Felix Fietkau <nbd@....name>, Sean Wang <sean.wang@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
"Chester A. Unal" <chester.a.unal@...nc9.com>,
Daniel Golle <daniel@...rotopia.org>,
DENG Qingfang <dqfext@...il.com>, Andrew Lunn <andrew@...n.ch>,
Vladimir Oltean <olteanv@...il.com>, netdev@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
upstream@...oha.com
Subject: Re: [PATCH net-next v4 16/16] net: airoha: Introduce PPE debugfs
support
On Feb 17, Simon Horman wrote:
> On Thu, Feb 13, 2025 at 04:34:35PM +0100, Lorenzo Bianconi wrote:
> > Similar to PPE support for Mediatek devices, introduce PPE debugfs
> > in order to dump binded and unbinded flows.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
>
> ...
>
> > diff --git a/drivers/net/ethernet/airoha/airoha_ppe_debugfs.c b/drivers/net/ethernet/airoha/airoha_ppe_debugfs.c
>
> ...
>
> > +static int airoha_ppe_debugfs_foe_show(struct seq_file *m, void *private,
> > + bool bind)
> > +{
> > + static const char *const ppe_type_str[] = {
> > + [PPE_PKT_TYPE_IPV4_HNAPT] = "IPv4 5T",
> > + [PPE_PKT_TYPE_IPV4_ROUTE] = "IPv4 3T",
> > + [PPE_PKT_TYPE_BRIDGE] = "L2B",
> > + [PPE_PKT_TYPE_IPV4_DSLITE] = "DS-LITE",
> > + [PPE_PKT_TYPE_IPV6_ROUTE_3T] = "IPv6 3T",
> > + [PPE_PKT_TYPE_IPV6_ROUTE_5T] = "IPv6 5T",
> > + [PPE_PKT_TYPE_IPV6_6RD] = "6RD",
> > + };
> > + static const char *const ppe_state_str[] = {
> > + [AIROHA_FOE_STATE_INVALID] = "INV",
> > + [AIROHA_FOE_STATE_UNBIND] = "UNB",
> > + [AIROHA_FOE_STATE_BIND] = "BND",
> > + [AIROHA_FOE_STATE_FIN] = "FIN",
> > + };
> > + struct airoha_ppe *ppe = m->private;
> > + int i;
> > +
> > + for (i = 0; i < PPE_NUM_ENTRIES; i++) {
> > + const char *state_str, *type_str = "UNKNOWN";
> > + u16 *src_port = NULL, *dest_port = NULL;
> > + struct airoha_foe_mac_info_common *l2;
> > + unsigned char h_source[ETH_ALEN] = {};
> > + unsigned char h_dest[ETH_ALEN];
> > + struct airoha_foe_entry *hwe;
> > + u32 type, state, ib2, data;
> > + void *src_addr, *dest_addr;
> > + bool ipv6 = false;
> > +
> > + hwe = airoha_ppe_foe_get_entry(ppe, i);
> > + if (!hwe)
> > + continue;
> > +
> > + state = FIELD_GET(AIROHA_FOE_IB1_STATE, hwe->ib1);
> > + if (!state)
> > + continue;
> > +
> > + if (bind && state != AIROHA_FOE_STATE_BIND)
> > + continue;
> > +
> > + state_str = ppe_state_str[state % ARRAY_SIZE(ppe_state_str)];
> > + type = FIELD_GET(AIROHA_FOE_IB1_PACKET_TYPE, hwe->ib1);
> > + if (type < ARRAY_SIZE(ppe_type_str) && ppe_type_str[type])
> > + type_str = ppe_type_str[type];
> > +
> > + seq_printf(m, "%05x %s %7s", i, state_str, type_str);
> > +
> > + switch (type) {
> > + case PPE_PKT_TYPE_IPV4_HNAPT:
> > + case PPE_PKT_TYPE_IPV4_DSLITE:
> > + src_port = &hwe->ipv4.orig_tuple.src_port;
> > + dest_port = &hwe->ipv4.orig_tuple.dest_port;
> > + fallthrough;
> > + case PPE_PKT_TYPE_IPV4_ROUTE:
> > + src_addr = &hwe->ipv4.orig_tuple.src_ip;
> > + dest_addr = &hwe->ipv4.orig_tuple.dest_ip;
> > + break;
> > + case PPE_PKT_TYPE_IPV6_ROUTE_5T:
> > + src_port = &hwe->ipv6.src_port;
> > + dest_port = &hwe->ipv6.dest_port;
> > + fallthrough;
> > + case PPE_PKT_TYPE_IPV6_ROUTE_3T:
> > + case PPE_PKT_TYPE_IPV6_6RD:
> > + src_addr = &hwe->ipv6.src_ip;
> > + dest_addr = &hwe->ipv6.dest_ip;
> > + ipv6 = true;
> > + break;
> > + }
>
> Hi Lorenzo,
Hi Simon,
>
> Perhaps it can't happen, but if type is not one of the cases handled
> by the switch statement above then src_addr and dest_addr will
> be used while uninitialised by the call to airoha_debugfs_ppe_print_tuple()
> below.
ack, right. I will fix it in v6.
Regards,
Lorenzo
>
> Flagged by Smatch.
>
> > +
> > + seq_puts(m, " orig=");
> > + airoha_debugfs_ppe_print_tuple(m, src_addr, dest_addr,
> > + src_port, dest_port, ipv6);
> > +
> > + switch (type) {
> > + case PPE_PKT_TYPE_IPV4_HNAPT:
> > + case PPE_PKT_TYPE_IPV4_DSLITE:
> > + src_port = &hwe->ipv4.new_tuple.src_port;
> > + dest_port = &hwe->ipv4.new_tuple.dest_port;
> > + fallthrough;
> > + case PPE_PKT_TYPE_IPV4_ROUTE:
> > + src_addr = &hwe->ipv4.new_tuple.src_ip;
> > + dest_addr = &hwe->ipv4.new_tuple.dest_ip;
> > + seq_puts(m, " new=");
> > + airoha_debugfs_ppe_print_tuple(m, src_addr, dest_addr,
> > + src_port, dest_port,
> > + ipv6);
> > + break;
> > + }
> > +
> > + if (type >= PPE_PKT_TYPE_IPV6_ROUTE_3T) {
> > + data = hwe->ipv6.data;
> > + ib2 = hwe->ipv6.ib2;
> > + l2 = &hwe->ipv6.l2;
> > + } else {
> > + data = hwe->ipv4.data;
> > + ib2 = hwe->ipv4.ib2;
> > + l2 = &hwe->ipv4.l2.common;
> > + *((__be16 *)&h_source[4]) =
> > + cpu_to_be16(hwe->ipv4.l2.src_mac_lo);
> > + }
> > +
> > + *((__be32 *)h_dest) = cpu_to_be32(l2->dest_mac_hi);
> > + *((__be16 *)&h_dest[4]) = cpu_to_be16(l2->dest_mac_lo);
> > + *((__be32 *)h_source) = cpu_to_be32(l2->src_mac_hi);
> > +
> > + seq_printf(m, " eth=%pM->%pM etype=%04x data=%08x"
> > + " vlan=%d,%d ib1=%08x ib2=%08x\n",
> > + h_source, h_dest, l2->etype, data,
> > + l2->vlan1, l2->vlan2, hwe->ib1, ib2);
> > + }
> > +
> > + return 0;
> > +}
>
> ...
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists