Re: [PATCH net-next v2 3/7] net: dsa: Register devlink ports before calling DSA driver setup()
From: Andrew Lunn <andrew@lunn.ch>
Date: 2020-09-27 00:45:19
quoted
+static int dsa_port_devlink_setup(struct dsa_port *dp) { struct devlink_port *dlp = &dp->devlink_port; + struct dsa_switch_tree *dst = dp->ds->dst; + struct devlink_port_attrs attrs = {}; + struct devlink *dl = dp->ds->devlink; + const unsigned char *id; + unsigned char len; + int err; + + id = (const unsigned char *)&dst->index; + len = sizeof(dst->index); + + attrs.phys.port_number = dp->index; + memcpy(attrs.switch_id.id, id, len); + attrs.switch_id.id_len = len; + + if (dp->setup) + return 0;I wonder what this is protecting against? I ran on a multi-switch tree without these 2 lines and I didn't get anything like multiple registration or things like that. What is the call path that would call dsa_port_devlink_setup twice?
I made a duplicate copy of dsa_port_setup() and trimmed out what was not needed to give the new dsa_port_setup() and dsa_port_devlink_setup(). I did not trim enough...
quoted
+ switch (dp->type) { + case DSA_PORT_TYPE_UNUSED: + memset(dlp, 0, sizeof(*dlp)); + attrs.flavour = DEVLINK_PORT_FLAVOUR_UNUSED;quoted
+ devlink_port_attrs_set(dlp, &attrs); + err = devlink_port_register(dl, dlp, dp->index);These 2 lines are common everywhere. Could you move them out of the switch-case statement?
Yes, that makes sense. Too much blind copy/paste without actually reviewing the code afterwards. Andrew