Re: [iproute2] iplink: add support for IFLA_CARRIER attribute
On Thu, 27 Apr 2017 14:35:01 +0800
张胜举 wrote:
> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Wednesday, April 26, 2017 11:08 PM
> > To: Zhang Shengju
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [iproute2] iplink: add support for IFLA_CARRIER attribute
> >
> > On Wed, 26 Apr 2017 15:08:39 +0800
> > Zhang Shengju wrote:
> >
> > > Add support to set IFLA_CARRIER attribute.
> > >
> > > Signed-off-by: Zhang Shengju
> > > ---
> > > ip/iplink.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/ip/iplink.c b/ip/iplink.c index 866ad72..263bfdd 100644
> > > --- a/ip/iplink.c
> > > +++ b/ip/iplink.c
> > > @@ -72,6 +72,7 @@ void iplink_usage(void)
> > > " [ allmulticast { on | off } ]\n"
> > > " [ promisc { on | off } ]\n"
> > > " [ trailers { on | off } ]\n"
> > > + " [ carrier { on | off } ]\n"
> > > " [ txqueuelen PACKETS ]\n"
> > > " [ name NEWNAME ]\n"
> > > " [ address LLADDR ]\n"
> > > @@ -673,6 +674,17 @@ int iplink_parse(int argc, char **argv, struct
> > iplink_req *req,
> > > req->i.ifi_flags |= IFF_NOARP;
> > > else
> > > return on_off("arp", *argv);
> > > + } else if (strcmp(*argv, "carrier") == 0) {
> > > + int carrier;
> > > + NEXT_ARG();
> > > + if (strcmp(*argv, "on") == 0)
> > > + carrier = 1;
> > > + else if (strcmp(*argv, "off") == 0)
> > > + carrier = 0;
> > > + else
> > > + return on_off("carrier", *argv);
> > > +
> > > + addattr8(&req->n, sizeof(*req), IFLA_CARRIER,
> > carrier);
> > > } else if (strcmp(*argv, "vf") == 0) {
> > > struct rtattr *vflist;
> > >
> >
> > The general policy of ip link command is all options should be invertable.
> Yes, so I add 'on' and 'off' subcommand to make sure that it can be
> invertable.
>
> > There are some VPN's that use this to save and restore state. So if you
> add
> > an option to set something there should be similar output under the
> detailed
> > show command.
> Currently, "show command" already can display 'carrier' status. Such as:
> dummy0:
>
Applied.
RE: [iproute2] iplink: add support for IFLA_CARRIER attribute
> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, April 26, 2017 11:08 PM
> To: Zhang Shengju
> Cc: netdev@vger.kernel.org
> Subject: Re: [iproute2] iplink: add support for IFLA_CARRIER attribute
>
> On Wed, 26 Apr 2017 15:08:39 +0800
> Zhang Shengju wrote:
>
> > Add support to set IFLA_CARRIER attribute.
> >
> > Signed-off-by: Zhang Shengju
> > ---
> > ip/iplink.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/ip/iplink.c b/ip/iplink.c index 866ad72..263bfdd 100644
> > --- a/ip/iplink.c
> > +++ b/ip/iplink.c
> > @@ -72,6 +72,7 @@ void iplink_usage(void)
> > " [ allmulticast { on | off } ]\n"
> > " [ promisc { on | off } ]\n"
> > " [ trailers { on | off } ]\n"
> > + " [ carrier { on | off } ]\n"
> > " [ txqueuelen PACKETS ]\n"
> > " [ name NEWNAME ]\n"
> > " [ address LLADDR ]\n"
> > @@ -673,6 +674,17 @@ int iplink_parse(int argc, char **argv, struct
> iplink_req *req,
> > req->i.ifi_flags |= IFF_NOARP;
> > else
> > return on_off("arp", *argv);
> > + } else if (strcmp(*argv, "carrier") == 0) {
> > + int carrier;
> > + NEXT_ARG();
> > + if (strcmp(*argv, "on") == 0)
> > + carrier = 1;
> > + else if (strcmp(*argv, "off") == 0)
> > + carrier = 0;
> > + else
> > + return on_off("carrier", *argv);
> > +
> > + addattr8(&req->n, sizeof(*req), IFLA_CARRIER,
> carrier);
> > } else if (strcmp(*argv, "vf") == 0) {
> > struct rtattr *vflist;
> >
>
> The general policy of ip link command is all options should be invertable.
Yes, so I add 'on' and 'off' subcommand to make sure that it can be
invertable.
> There are some VPN's that use this to save and restore state. So if you
add
> an option to set something there should be similar output under the
detailed
> show command.
Currently, "show command" already can display 'carrier' status. Such as:
dummy0:
Re: [iproute2] iplink: add support for IFLA_CARRIER attribute
On Wed, 26 Apr 2017 15:08:39 +0800
Zhang Shengju wrote:
> Add support to set IFLA_CARRIER attribute.
>
> Signed-off-by: Zhang Shengju
> ---
> ip/iplink.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 866ad72..263bfdd 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -72,6 +72,7 @@ void iplink_usage(void)
> " [ allmulticast { on | off } ]\n"
> " [ promisc { on | off } ]\n"
> " [ trailers { on | off } ]\n"
> + " [ carrier { on | off } ]\n"
> " [ txqueuelen PACKETS ]\n"
> " [ name NEWNAME ]\n"
> " [ address LLADDR ]\n"
> @@ -673,6 +674,17 @@ int iplink_parse(int argc, char **argv, struct
> iplink_req *req,
> req->i.ifi_flags |= IFF_NOARP;
> else
> return on_off("arp", *argv);
> + } else if (strcmp(*argv, "carrier") == 0) {
> + int carrier;
> + NEXT_ARG();
> + if (strcmp(*argv, "on") == 0)
> + carrier = 1;
> + else if (strcmp(*argv, "off") == 0)
> + carrier = 0;
> + else
> + return on_off("carrier", *argv);
> +
> + addattr8(&req->n, sizeof(*req), IFLA_CARRIER, carrier);
> } else if (strcmp(*argv, "vf") == 0) {
> struct rtattr *vflist;
>
The general policy of ip link command is all options should be invertable.
There are some VPN's that use this to save and restore state. So if you add
an option to set something there should be similar output under the detailed
show command.
Re: [iproute2] iplink: add support for IFLA_CARRIER attribute
On 26/04/17 10:08, Zhang Shengju wrote:
> Add support to set IFLA_CARRIER attribute.
>
> Signed-off-by: Zhang Shengju
> ---
> ip/iplink.c | 12
> 1 file changed, 12 insertions(+)
>
You should also update the ip-link man page with this new option.
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 866ad72..263bfdd 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -72,6 +72,7 @@ void iplink_usage(void)
> " [ allmulticast { on | off } ]\n"
> " [ promisc { on | off } ]\n"
> " [ trailers { on | off } ]\n"
> + " [ carrier { on | off } ]\n"
> " [ txqueuelen PACKETS ]\n"
> " [ name NEWNAME ]\n"
> " [ address LLADDR ]\n"
> @@ -673,6 +674,17 @@ int iplink_parse(int argc, char **argv, struct
> iplink_req *req,
> req->i.ifi_flags |= IFF_NOARP;
> else
> return on_off("arp", *argv);
> + } else if (strcmp(*argv, "carrier") == 0) {
> + int carrier;
Please leave a blank line between the variable definition and the code.
> + NEXT_ARG();
> + if (strcmp(*argv, "on") == 0)
> + carrier = 1;
> + else if (strcmp(*argv, "off") == 0)
> + carrier = 0;
> + else
> + return on_off("carrier", *argv);
> +
> + addattr8(&req->n, sizeof(*req), IFLA_CARRIER, carrier);
> } else if (strcmp(*argv, "vf") == 0) {
> struct rtattr *vflist;
>
>
