Messages in this thread | | | Subject | Re: [RFC net-next 1/3] net: marvell: prestera: Add Switchdev driver for Prestera family ASIC device 98DX325x (AC3x) | From | Florian Fainelli <> | Date | Thu, 27 Feb 2020 20:22:02 -0800 |
| |
On 2/25/2020 8:30 AM, Vadym Kochan wrote: > Marvell Prestera 98DX326x integrates up to 24 ports of 1GbE with 8 > ports of 10GbE uplinks or 2 ports of 40Gbps stacking for a largely > wireless SMB deployment. > > This driver implementation includes only L1 & basic L2 support. > > The core Prestera switching logic is implemented in prestera.c, there is > an intermediate hw layer between core logic and firmware. It is > implemented in prestera_hw.c, the purpose of it is to encapsulate hw > related logic, in future there is a plan to support more devices with > different HW related configurations. > > The following Switchdev features are supported: > > - VLAN-aware bridge offloading > - VLAN-unaware bridge offloading > - FDB offloading (learning, ageing) > - Switchport configuration > > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> > Signed-off-by: Andrii Savka <andrii.savka@plvision.eu> > Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu> > Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu> > Signed-off-by: Serhiy Pshyk <serhiy.pshyk@plvision.eu> > Signed-off-by: Taras Chornyi <taras.chornyi@plvision.eu> > Signed-off-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
Very little to pick on, the driver is nice and clean, great job!
> ---
[snip]
> +#define PORT_STATS_CACHE_TIMEOUT_MS (msecs_to_jiffies(1000)) > +#define PORT_STATS_CNT (sizeof(struct mvsw_pr_port_stats) / sizeof(u64))
All entries in mvsw_pr_port_stats are u64 so you can use ARRAY_SIZE() here.
[snip]
> + > + err = register_netdev(net_dev); > + if (err) > + goto err_register_netdev; > + > + list_add(&port->list, &sw->port_list);
As soon as you publish the network device it can be used by notifiers, user-space etc, better do this as the last operation.
[snip]
> +int mvsw_pr_hw_port_stats_get(const struct mvsw_pr_port *port, > + struct mvsw_pr_port_stats *stats) > +{ > + struct mvsw_msg_port_stats_ret resp; > + struct mvsw_msg_port_attr_cmd req = { > + .attr = MVSW_MSG_PORT_ATTR_STATS, > + .port = port->hw_id, > + .dev = port->dev_id > + }; > + u64 *hw_val = resp.stats; > + int err; > + > + err = fw_send_req_resp(port->sw, MVSW_MSG_TYPE_PORT_ATTR_GET, > + &req, &resp); > + if (err) > + return err; > + > + stats->good_octets_received = hw_val[MVSW_PORT_GOOD_OCTETS_RCV_CNT];
This seems error prone and not scaling really well, since all stats member are u64 and they are ordered in the same way as the response, is not a memcpy() sufficient here? -- Florian
| |