From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: subashab@codeaurora.org
Cc: Alex Elder <elder@linaro.org>,
Sean Tranchetti <stranche@codeaurora.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Daniele Palmas <dnlplm@gmail.com>,
Aleksander Morgado <aleksander@aleksander.es>,
Loic Poulain <loic.poulain@linaro.org>
Subject: Re: [PATCH] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
Date: Thu, 22 Apr 2021 21:30:26 -0500 [thread overview]
Message-ID: <20210423023026.GD1908499@yoga> (raw)
In-Reply-To: <89526b9845cc86143da2221fc2445557@codeaurora.org>
On Thu 22 Apr 18:28 CDT 2021, subashab@codeaurora.org wrote:
> On 2021-04-22 12:29, Alex Elder wrote:
> > On 4/22/21 1:20 PM, Bjorn Andersson wrote:
> > > The idiomatic way to handle the changelink flags/mask pair seems to be
> > > allow partial updates of the driver's link flags. In contrast the
> > > rmnet
> > > driver masks the incoming flags and then use that as the new flags.
> > >
> > > Change the rmnet driver to follow the common scheme, before the
> > > introduction of IFLA_RMNET_FLAGS handling in iproute2 et al.
> >
> > I like this a lot. It should have been implemented this way
> > to begin with; there's not much point to have the mask if
> > it's only applied to the passed-in value.
> >
> > KS, are you aware of *any* existing user space code that
> > would not work correctly if this were accepted?
> >
> > I.e., the way it was (is), the value passed in *assigns*
> > the data format flags. But with Bjorn's changes, the
> > data format flags would be *updated* (i.e., any bits not
> > set in the mask field would remain with their previous
> > value).
> >
> > Reviewed-by: Alex Elder <elder@linaro.org>
>
> What rmnet functionality which was broken without this change.
> That doesnt seem to be listed in this patch commit text.
>
I recently posted a patch to iproute2 extending the rmnet link handling
to handle IFLA_RMNET_FLAGS, in the discussion that followed this subject
came up. So nothing is broken, it's just that the current logic doesn't
make sense and I wanted to attempt to fix it before we start to use it
commonly distributed userspace software (iproute2, libqmi etc)
> If this is an enhancement, then patch needs to be targeted to net-next
> instead of net
Okay, please let me know what hoops you want me to jump through. I just
want the subject concluded so that I can respin my iproute2 patch
according to what we decide here.
Regards,
Bjorn
next prev parent reply other threads:[~2021-04-23 2:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-22 18:20 [PATCH] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS Bjorn Andersson
2021-04-22 18:29 ` Alex Elder
2021-04-22 23:28 ` subashab
2021-04-23 1:01 ` Alex Elder
2021-04-23 2:30 ` Bjorn Andersson [this message]
2021-04-23 4:04 ` subashab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210423023026.GD1908499@yoga \
--to=bjorn.andersson@linaro.org \
--cc=aleksander@aleksander.es \
--cc=davem@davemloft.net \
--cc=dnlplm@gmail.com \
--cc=elder@linaro.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=stranche@codeaurora.org \
--cc=subashab@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox