On Tue, Apr 18, 2023 at 3:16 PM wangyunjian <wangyunj...@huawei.com> wrote:
> > I wonder though if we should pass any ofpbuf at all, as we don't care about 
> > the
> > reply from the kernel for this case.
> > Something like:
> >
> > $ git diff
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 
> > de07112778..60bd39643c
> > 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -395,7 +395,7 @@ dpif_netlink_open(const struct dpif_class *class
> > OVS_UNUSED, const char *name,
> >      dp_request.user_features |= OVS_DP_F_UNALIGNED;
> >      dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
> >      dp_request.user_features |= OVS_DP_F_UNSUPPORTED;
> > -    error = dpif_netlink_dp_transact(&dp_request, &dp, &buf);
> > +    error = dpif_netlink_dp_transact(&dp_request, NULL, NULL);
> >      if (error) {
> >          /* The Open vSwitch kernel module has two modes for dispatching
> >           * upcalls: per-vport and per-cpu.
> >
> > WDYT?
>
> Currently, the 'buf' will check whether it is NULL in the 
> dpif_netlink_dp_transact().
> And it will be used in dpif_netlink_dp_from_ofpbuf().

- Looking at dpif_netlink_dp_transact() comments.

/* Executes 'request' in the kernel datapath.  If the command fails, returns a
 * positive errno value.  Otherwise, if 'reply' and 'bufp' are null, returns 0
 * without doing anything else.  If 'reply' and 'bufp' are nonnull, then the
 * result of the command is expected to be of the same form, which is decoded
 * and stored in '*reply' and '*bufp'.  The caller must free '*bufp' when the
 * reply is no longer needed ('reply' will contain pointers into '*bufp'). */
static int
dpif_netlink_dp_transact(const struct dpif_netlink_dp *request,
                         struct dpif_netlink_dp *reply, struct ofpbuf **bufp)

I understand that dpif_netlink_dp_transact() makes it possible for a
caller to simply ignore the reply from the kernel by passing both
'reply' and 'bufp' as NULL.


- In the specific call to dpif_netlink_dp_transact() (line 398) in
dpif_netlink_open(), the 'dp' content is not considered in the branch
when no error returned (starting line 430).
Besides, 'dp' / 'buf' are overwritten later in this same branch, by
sending a new netlink request (line 437).

So if this code is not going to use 'dp' and 'buf' content, why not
simply pass NULL/NULL?


-- 
David Marchand

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to