Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BB406C433FE for ; Tue, 7 Dec 2021 02:25:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241764AbhLGC2b (ORCPT ); Mon, 6 Dec 2021 21:28:31 -0500 Received: from ams.source.kernel.org ([145.40.68.75]:59584 "EHLO ams.source.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241497AbhLGC23 (ORCPT ); Mon, 6 Dec 2021 21:28:29 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 31A9DB81611; Tue, 7 Dec 2021 02:24:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA889C004DD; Tue, 7 Dec 2021 02:24:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638843898; bh=je9VRaZCoX11UWGvRaPQFHeg/OsHxIwbxim2ugr49vo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Afb4fAcK8BGPwbBVQGdcOzaZNRy39zXYamRkFLCFP65Jdfk7Psl1hWLpCU6vdrp0M iHifziGQyr/oT6+xq8DIGNu/2iHGUxMjk7xdjAnw6H7vn2VgIurCfzIvUw6bn8IoaL /Wx6Z5Q11xdbiVIDHY7IaMUDaujGs2iqgAFNchBy1rbV7rsa2na3n9Jmu3t1udp8Hf 82rzTFTk6y2DUpeJ0Ti8SXEJ2s3vSM9oboa8zRXKPioyF75n5nNbrTQYZuiH+u5Otj ciuGJlRCfcTOZ50zX9CFdISA3h5daYpi95pmd5mtwuiLrsHY1oX74CcPLDkdOyrZSB RhMlBhzee0Vbw== Date: Mon, 6 Dec 2021 18:24:56 -0800 From: Jakub Kicinski To: Horatiu Vultur Cc: , , , , , , Subject: Re: [PATCH net-next 3/6] net: lan966x: add support for interrupts from analyzer Message-ID: <20211206182456.4494c5f6@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: <20211203104645.1476704-4-horatiu.vultur@microchip.com> References: <20211203104645.1476704-1-horatiu.vultur@microchip.com> <20211203104645.1476704-4-horatiu.vultur@microchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > +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. Also why GFP_ATOMIC? Memory allocations are _a lot_ less likely with GFP_KERNEL. > + 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; > + } > + }