[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMArcTW_dgUVjMD5+yUPQrJxjQOaGyHJ0DT1Kx6m=fGeMrA2=g@mail.gmail.com>
Date: Thu, 27 Feb 2020 16:36:01 +0900
From: Taehee Yoo <ap420073@...il.com>
To: subashab@...eaurora.org
Cc: David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, stranche@...eaurora.org,
Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net 06/10] net: rmnet: print error message when command fails
On Thu, 27 Feb 2020 at 05:37, <subashab@...eaurora.org> wrote:
>
Hi,
Thank you for the review :)
> On 2020-02-26 10:47, Taehee Yoo wrote:
> > When rmnet netlink command fails, it doesn't print any error message.
> > So, users couldn't know the exact reason.
> > In order to tell the exact reason to the user, the extack error message
> > is used in this patch.
> >
> > Signed-off-by: Taehee Yoo <ap420073@...il.com>
> > ---
> > .../ethernet/qualcomm/rmnet/rmnet_config.c | 26 ++++++++++++-------
> > .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 10 +++----
> > .../net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 3 ++-
> > 3 files changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> > b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> > index c8b1bfe127ac..93745cd45c29 100644
> > --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> > +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> > @@ -141,11 +141,10 @@ static int rmnet_newlink(struct net *src_net,
> > struct net_device *dev,
> > }
> >
> > real_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> > - if (!real_dev || !dev)
> > + if (!real_dev || !dev) {
> > + NL_SET_ERR_MSG_MOD(extack, "link does not exist");
> > return -ENODEV;
> > -
> > - if (!data[IFLA_RMNET_MUX_ID])
> > - return -EINVAL;
> > + }
> >
> > ep = kzalloc(sizeof(*ep), GFP_ATOMIC);
> > if (!ep)
> > @@ -158,7 +157,7 @@ static int rmnet_newlink(struct net *src_net,
> > struct net_device *dev,
> > goto err0;
> >
> > port = rmnet_get_port_rtnl(real_dev);
> > - err = rmnet_vnd_newlink(mux_id, dev, port, real_dev, ep);
> > + err = rmnet_vnd_newlink(mux_id, dev, port, real_dev, ep, extack);
> > if (err)
> > goto err1;
> >
> > @@ -275,12 +274,16 @@ static int rmnet_rtnl_validate(struct nlattr
> > *tb[], struct nlattr *data[],
> > {
> > u16 mux_id;
> >
> > - if (!data || !data[IFLA_RMNET_MUX_ID])
> > + if (!data || !data[IFLA_RMNET_MUX_ID]) {
> > + NL_SET_ERR_MSG_MOD(extack, "MUX ID not specifies");
> > return -EINVAL;
> > + }
> >
> > mux_id = nla_get_u16(data[IFLA_RMNET_MUX_ID]);
> > - if (mux_id > (RMNET_MAX_LOGICAL_EP - 1))
> > + if (mux_id > (RMNET_MAX_LOGICAL_EP - 1)) {
> > + NL_SET_ERR_MSG_MOD(extack, "invalid MUX ID");
> > return -ERANGE;
> > + }
> >
> > return 0;
> > }
> > @@ -414,11 +417,16 @@ int rmnet_add_bridge(struct net_device
> > *rmnet_dev,
> > /* If there is more than one rmnet dev attached, its probably being
> > * used for muxing. Skip the briding in that case
> > */
> > - if (port->nr_rmnet_devs > 1)
> > + if (port->nr_rmnet_devs > 1) {
> > + NL_SET_ERR_MSG_MOD(extack, "more than one rmnet dev attached");
> > return -EINVAL;
> > + }
> >
> > - if (rmnet_is_real_dev_registered(slave_dev))
> > + if (rmnet_is_real_dev_registered(slave_dev)) {
> > + NL_SET_ERR_MSG_MOD(extack,
> > + "dev is already attached another rmnet dev");
> > return -EBUSY;
> > + }
> >
> > err = rmnet_register_real_device(slave_dev);
> > if (err)
> > diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> > b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> > index a26e76e9d382..90c19033ebe0 100644
> > --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> > +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> > @@ -224,16 +224,16 @@ void rmnet_vnd_setup(struct net_device
> > *rmnet_dev)
> > int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
> > struct rmnet_port *port,
> > struct net_device *real_dev,
> > - struct rmnet_endpoint *ep)
> > + struct rmnet_endpoint *ep,
> > + struct netlink_ext_ack *extack)
> > {
> > struct rmnet_priv *priv = netdev_priv(rmnet_dev);
> > int rc;
> >
> > - if (ep->egress_dev)
> > - return -EINVAL;
> > -
> > - if (rmnet_get_endpoint(port, id))
> > + if (rmnet_get_endpoint(port, id)) {
> > + NL_SET_ERR_MSG_MOD(extack, "MUX ID already exists");
> > return -EBUSY;
> > + }
> >
> > rmnet_dev->hw_features = NETIF_F_RXCSUM;
> > rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> > diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> > b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> > index 54cbaf3c3bc4..d8fa76e8e9c4 100644
> > --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> > +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> > @@ -11,7 +11,8 @@ int rmnet_vnd_do_flow_control(struct net_device
> > *dev, int enable);
> > int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
> > struct rmnet_port *port,
> > struct net_device *real_dev,
> > - struct rmnet_endpoint *ep);
> > + struct rmnet_endpoint *ep,
> > + struct netlink_ext_ack *extack);
> > int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
> > struct rmnet_endpoint *ep);
> > void rmnet_vnd_rx_fixup(struct sk_buff *skb, struct net_device *dev);
>
> This patch and [PATCH net 02/10] "net: rmnet: add missing module alias"
> seem
> to be adding new functionality. Perhaps it can be sent to net-next
> instead
> of net.
Okay, I will drop these two patches from this patchset.
And I will send these two patches to net-next separately.
Thanks a lot!
Taehee Yoo
Powered by blists - more mailing lists