On Thu, Feb 2, 2017 at 9:18 AM, Mintz, Yuval <yuval.mi...@cavium.com> wrote:
>> +config BNXT_XDP
>> + bool "Xpress Data Path (XDP) driver support"
>> + default n
>> + depends on BNXT && BPF
>> + ---help---
>> + Say Y here if you want to enable XDP in the driver to support
>> + eBPF programs in the fast path.
>> +
>
> Wasn't it recently discussed that per-feature option is preferable
> to a per-feature per-device option?
> Assuming BPF > XDP and thus shouldn't directly imply that XDP
> should be supported, perhaps the right thing is to add a global
> XDP config option?
Since the driver has other per-device options, I'm adding another
per-device option right now. It can easily be converted to a more
tree-wide option if that's what we want to do.
>
>> + if (prog && bp->dev->mtu > BNXT_MAX_PAGE_MODE_MTU) {
>> + netdev_err(dev, "MTU %d larger than largest XDP supported
>> MTU %d.\n",
>> + bp->dev->mtu, BNXT_MAX_PAGE_MODE_MTU);
>> + return -EOPNOTSUPP;
>> + }
>
> Is it O.k. to print with netdev_err() for a user-provided unsupported
> configuration? Shouldn't that be limited?
You are suggesting to use lower level warning, such as netdev_warn(), right?
>
>> + bool sh = (bp->flags & BNXT_FLAG_SHARED_RINGS) ? true :
>> false;
>
> Didn't you already check this flag is set?
>
You are right. I checked already.
