Re: [PATCH v1 3/8] sunrpc: add a directory per sunrpc xprt

[Dan Aloni <dan@kernelim.com>]

On Tue, Feb 16, 2021 at 04:46:55PM -0500, Anna Schumaker wrote:
> > +static ssize_t rpc_netns_xprt_dstaddr_show(struct kobject *kobj,
> > +               struct kobj_attribute *attr, char *buf)
> > +{
> > +       struct rpc_netns_xprt *c = container_of(kobj,
> > +                               struct rpc_netns_xprt, kobject);
> > +       struct rpc_xprt *xprt = c->xprt;
> > +
> > +       if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA))) {
> 
> We might want to change these restrictions later on, so if we're going
> to put this into each function then maybe it would make sense to have
> a quick inline to check protocol support?

Yeah, I agree.

> I do the same check in the setup function for my patches, so if you
> want I can add the inline function and then it'll just be there for
> you to use.

Sure, go ahead.

> 
> > +               sprintf(buf, "N/A");
> > +               return 0;
> 
> I'm guessing the point of putting "N/A" here is so userspace tools
> don't have to guess which files exist or not for each protocol type?
> Should I change my patches to match this style too?

Yes, though I'm not sure what are the common kernel convention here.
Maybe a "-" string is sufficient?

-- 
Dan Aloni