RE: [PATCH] qed: fix qed_fill_link() error handling
From: Yuval Mintz <hidden>
Date: 2016-06-01 11:10:43
Also in:
lkml
quoted
quoted
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 doI 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