Messages in this thread | | | Subject | Re: [PATCH net] net: dsa: Do not leave DSA master with NULL netdev_ops | From | Florian Fainelli <> | Date | Mon, 4 May 2020 13:40:32 -0700 |
| |
On 5/4/2020 1:34 PM, Vladimir Oltean wrote: > Hi Florian, > > On Mon, 4 May 2020 at 23:19, Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> When ndo_get_phys_port_name() for the CPU port was added we introduced >> an early check for when the DSA master network device in >> dsa_master_ndo_setup() already implements ndo_get_phys_port_name(). When >> we perform the teardown operation in dsa_master_ndo_teardown() we would >> not be checking that cpu_dp->orig_ndo_ops was successfully allocated and >> non-NULL initialized. >> >> With network device drivers such as virtio_net, this leads to a NPD as >> soon as the DSA switch hanging off of it gets torn down because we are >> now assigning the virtio_net device's netdev_ops a NULL pointer. >> >> Fixes: da7b9e9b00d4 ("net: dsa: Add ndo_get_phys_port_name() for CPU port") >> Reported-by: Allen Pais <allen.pais@oracle.com> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- > > The fix makes complete sense. > But on another note, if we don't overlay an ndo_get_phys_port_name if > the master already has one, doesn't that render the entire mechanism > of having a reliable way for user space to determine the CPU port > number pointless?
For the CPU port I would consider ndo_get_phys_port_name() to be more best effort than an absolute need unlike the user facing ports, where this is necessary for a variety of actions (e.g.: determining queues/port numbers etc.) which is why there was no overlay being done in that case. There is not a good way to cascade the information other than do something like pX.Y and defining what the X and Y are, what do you think? -- Florian
| |