Re: [RFC PATCH net-next 2/2] net: dcb: add new apptrust attribute

Hi Petr,

[off-list ref] writes:

quoted

Petr Machata [off-list ref] writes:

quoted

But of course this will never get anywhere close to that. We will end up
passing maybe one, two entries. So the UAPI seems excessive in how it
hands around this large array.

I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE
payload be an array of bytes, each byte a selector? Or something similar
to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of
payload attributes?

Hmm. It might seem excessive, but a quick few thoughts on your proposed solution:
  - We need more code to define and parse the new DCB_ATTR_IEEE_APP_TRUST_TABLE /
    DCB_ATTR_IEEE_APP_TRUST attributes.

Yes, a bit. But it's not too bad IMHO. Am I forgetting something here?

        u8 selectors[256];
        int nselectors;
        int rem;

        nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE], rem) {
                if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
                    nla_len(attr) != 1 ||
                    nselectors >= sizeof(selectors)) {
                        err = -EINVAL;
                        goto err;
                }

                selectors[nselectors++] = nla_get_u8(attr);
        }

... and you have reconstructed the array.

LGTM.

quoted

  - If the selectors are passed individually to the driver, we need a
    dcbnl_delapptrust(), because now, the driver have to add and del from the
    driver maintained array. You could of course accumulate selectors in an array
    before passing them to the driver, but then why go away from the array in the
    first place.

I have no problem with using an array for the in-kernel API. There it's
easy to change. UAPI can't ever change.

quoted

quoted

quoted

+             struct ieee_apptrust *trust =
+                     nla_data(ieee[DCB_ATTR_IEEE_APP_TRUST]);

Besides invoking the OP, this should validate the payload. E.g. no
driver is supposed to accept trust policies that contain invalid
selectors. Pretty sure there's no value in repeated entries either.

Validation (bogus input and unique selectors) is done in userspace (dcb-apptrust).

Using iproute2 dcb is not mandatory, the UAPI is client-agnostic. The
kernel needs to bounce bogons as well. Otherwise they will become part
of the UAPI with the meaning "this doesn't do anything".

And yeah, drivers will validate supported configurations. But still the
requests that go to the driver should already be sanitized, so that the
driver code doesn't need to worry about this.

Good point.

Will prepare a v2 with suggested changes.