RE: [PATCH] qed: fix qed_fill_link() error handling

[Yuval Mintz]

> > > I think we can just remove the IS_ENABLED() check there and define
> > > the
> > > IS_PF() macro conditionally to become 'true' if CONFIG_QED_SRIOV is
> > > not set, like some other drivers do
> >
> > I think that would be unsafe with current qede - qede currently
> > publishes its VFs' PCI device-id as part its MODULE_DEVICE_TABLE, even
> > if CONFIG_QED_SRIOV isn't enabled [might be the wrong thing to do, but
> > that how it goes].
> > Without changing this, if for some reason we'd have an assigned VF to
> > a VM whose kernel isn't compiled with CONFIG_QED_SRIOV [which is an
> > odd config], that VM is likely to miserably crash.
> 
> Wouldn't it crash anyway if the code to handle VF devices is not present?
> E.g. the warning we got here tells us that qed_get_link_data() operates on
> uninitialized data when called on a VF device and SRIOV support is not built into
> the driver. I haven't looked if all the other functions handle that right, but my
> guess is that there are other functions with similar problems.
> 
> Maybe it's best to remove the PCI IDs fort the virtual devices from the table if
> they are not supported by the configuration.

Actually, I think VF probe should gracefully fail in that case,
as qed_vf_hw_prepare() would simply return -EINVAL.
But I can honestly say I've never tested this flow, and I agree there's
no reason to allow VF probe in case we're not supporting SRIOV.

So I guess removing the PCI ID and defining IS_PF to be true in case
CONFIG_QED_SRIOV isn't set is the right way to go.
Do you want to revise your patch, or do you want me to do it?

Thanks,
Yuval