Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411104

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

Show all headers | View raw

> > > 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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH] qed: fix qed_fill_link() error handling David Miller <davem@davemloft.net> - 2016-05-31 23:30 +0200
  Re: [PATCH] qed: fix qed_fill_link() error handling Arnd Bergmann <arnd@arndb.de> - 2016-06-01 00:40 +0200
    RE: [PATCH] qed: fix qed_fill_link() error handling Yuval Mintz <Yuval.Mintz@qlogic.com> - 2016-06-01 13:00 +0200
      Re: [PATCH] qed: fix qed_fill_link() error handling Arnd Bergmann <arnd@arndb.de> - 2016-06-01 13:10 +0200
        RE: [PATCH] qed: fix qed_fill_link() error handling Yuval Mintz <Yuval.Mintz@qlogic.com> - 2016-06-01 13:20 +0200
          [PATCH v2] qed: fix qed_fill_link() error handling Arnd Bergmann <arnd@arndb.de> - 2016-06-01 15:30 +0200
            RE: [PATCH v2] qed: fix qed_fill_link() error handling Yuval Mintz <Yuval.Mintz@qlogic.com> - 2016-06-01 15:40 +0200
              Re: [PATCH v2] qed: fix qed_fill_link() error handling David Miller <davem@davemloft.net> - 2016-06-02 07:10 +0200

csiph-web