From: Horatiu Vultur <horatiu.vultur@microchip.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: <davem@davemloft.net>, <robh+dt@kernel.org>,
<UNGLinuxDriver@microchip.com>, <linux@armlinux.org.uk>,
<netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 3/6] net: lan966x: add support for interrupts from analyzer
Date: Tue, 7 Dec 2021 13:03:05 +0100 [thread overview]
Message-ID: <20211207120305.atm3xhyhz4xl7vqw@soft-dev3-1.localhost> (raw)
In-Reply-To: <20211206182456.4494c5f6@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
The 12/06/2021 18:24, Jakub Kicinski wrote:
Hi Jakub,
>
> On Fri, 3 Dec 2021 11:46:42 +0100 Horatiu Vultur wrote:
> > This patch adds support for handling the interrupts generated by the
> > analyzer. Currently, only the MAC table generates these interrupts.
> > The MAC table will generate an interrupt whenever it learns or forgets
> > an entry in the table. It is the SW responsibility figure out which
> > entries were added/removed.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>
> > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(struct lan966x *lan966x,
> > + const unsigned char *mac,
> > + u16 vid, u16 port_index)
> > +{
> > + struct lan966x_mac_entry *mac_entry;
> > +
> > + mac_entry = devm_kzalloc(lan966x->dev,
> > + sizeof(*mac_entry), GFP_ATOMIC);
>
> Is it really necessary to use devm_ allocation for the mac entries?
> It's 2x memory overhead.
It is not necessary.
>
> Also why GFP_ATOMIC? Memory allocations are _a lot_ less likely with
> GFP_KERNEL.
Initially I thought this is called also in some context where it
couldn't sleep. But I was wrong.
I will change these in the next version.
>
> > + if (!mac_entry)
> > + return NULL;
> > +
> > + memcpy(mac_entry->mac, mac, ETH_ALEN);
> > + mac_entry->vid = vid;
> > + mac_entry->port_index = port_index;
> > + mac_entry->row = LAN966X_MAC_INVALID_ROW;
> > + return mac_entry;
> > +}
>
> > +static void lan966x_mac_process_raw_entry(struct lan966x_mac_raw_entry *raw_entry,
> > + u8 *mac, u16 *vid, u32 *dest_idx)
> > +{
> > + mac[0] = (raw_entry->mach >> 8) & 0xff;
> > + mac[1] = (raw_entry->mach >> 0) & 0xff;
> > + mac[2] = (raw_entry->macl >> 24) & 0xff;
> > + mac[3] = (raw_entry->macl >> 16) & 0xff;
> > + mac[4] = (raw_entry->macl >> 8) & 0xff;
> > + mac[5] = (raw_entry->macl >> 0) & 0xff;
> > +
> > + *vid = (raw_entry->mach >> 16) & 0xfff;
> > + *dest_idx = ANA_MACACCESS_DEST_IDX_GET(raw_entry->maca);
>
> Double space before =
>
> > +}
> > +
> > +static void lan966x_mac_irq_process(struct lan966x *lan966x, u32 row,
> > + struct lan966x_mac_raw_entry *raw_entries)
> > +{
> > + struct lan966x_mac_entry *mac_entry, *tmp;
> > + unsigned long flags;
> > + char mac[ETH_ALEN];
> > + u32 dest_idx;
> > + u32 column;
> > + u16 vid;
> > +
> > + spin_lock_irqsave(&lan966x->mac_lock, flags);
> > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, list) {
> > + bool founded = false;
>
> s/founded/found/
>
> > + if (mac_entry->row != row)
> > + continue;
> > +
> > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) {
> > + /* All the valid entries are at the start of the row,
> > + * so when get one invalid entry it can just skip the
> > + * rest of the columns
> > + */
> > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca))
> > + break;
> > +
> > + lan966x_mac_process_raw_entry(&raw_entries[column],
> > + mac, &vid, &dest_idx);
> > + WARN_ON(dest_idx > lan966x->num_phys_ports);
> > +
> > + /* If the entry in SW is found, then there is nothing
> > + * to do
> > + */
> > + if (mac_entry->vid == vid &&
> > + ether_addr_equal(mac_entry->mac, mac) &&
>
> You need to add __aligned(2) to mac, ether_addr_equal() needs aligned
> arguments.
>
> > + mac_entry->port_index == dest_idx) {
> > + raw_entries[column].process = true;
> > + founded = true;
> > + break;
> > + }
> > + }
--
/Horatiu
next prev parent reply other threads:[~2021-12-07 12:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-03 10:46 [PATCH net-next 0/6] net: lan966x: Add switchdev and vlan support Horatiu Vultur
2021-12-03 10:46 ` [PATCH net-next 1/6] net: lan966x: Add registers that are used for switch and vlan functionality Horatiu Vultur
2021-12-03 10:46 ` [PATCH net-next 2/6] dt-bindings: net: lan966x: Extend with the analyzer interrupt Horatiu Vultur
2021-12-03 10:46 ` [PATCH net-next 3/6] net: lan966x: add support for interrupts from analyzer Horatiu Vultur
2021-12-07 2:24 ` Jakub Kicinski
2021-12-07 12:03 ` Horatiu Vultur [this message]
2021-12-03 10:46 ` [PATCH net-next 4/6] net: lan966x: More MAC table functionality Horatiu Vultur
2021-12-03 10:46 ` [PATCH net-next 5/6] net: lan966x: Add vlan support Horatiu Vultur
2021-12-03 10:46 ` [PATCH net-next 6/6] net: lan966x: Add switchdev support Horatiu Vultur
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211207120305.atm3xhyhz4xl7vqw@soft-dev3-1.localhost \
--to=horatiu.vultur@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).