Re: [OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Florian Fainelli]

On Jan 13, 2016 10:07 PM, "Stefan Rompf"  wrote:
>
> Hi,
>
> On Montag, 11. Januar 2016 23:45:39 Rafał Miłecki wrote:
>
> > > Any comments to this usage/syntax? It doesn't look too nice (this long
> > > quoted string as a value), but swconfig is strongly focused on simple
> > > values.
> >
> > One more RFC... anyone?
>
> I am not an OpenWRT core developer so I cannot comment on the niceness of
the
> interface ;-) The string does not allow to specify combinations like
autoneg
> on, advertise 10 and 1000, but not 100, but that would be a very esoteric
use
> case and it is about configuring a switch port and not an ethernet tester.
> IMHO for the normal use case, it allows to specify everything that is
needed.
>
> So for me it's all right.

Same here, does not really bother me to go through several command lines to
get the desired settings.

Thanks!
--
Florian
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Stefan Rompf]

Hi,

On Montag, 11. Januar 2016 23:45:39 Rafał Miłecki wrote:

> > Any comments to this usage/syntax? It doesn't look too nice (this long
> > quoted string as a value), but swconfig is strongly focused on simple
> > values.
> 
> One more RFC... anyone?

I am not an OpenWRT core developer so I cannot comment on the niceness of the 
interface ;-) The string does not allow to specify combinations like autoneg 
on, advertise 10 and 1000, but not 100, but that would be a very esoteric use 
case and it is about configuring a switch port and not an ethernet tester. 
IMHO for the normal use case, it allows to specify everything that is needed.

So for me it's all right.

Stefan
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Rafał Miłecki]

On 7 January 2016 at 07:53, Rafał Miłecki  wrote:
> On 30 December 2015 at 12:10, Rafał Miłecki  wrote:
>> Supported syntax is inspired by ethtool. Example usage:
>> swconfig dev switch0 port 2 set link "duplex half speed 100 autoneg off"
>
> Any comments to this usage/syntax? It doesn't look too nice (this long
> quoted string as a value), but swconfig is strongly focused on simple
> values.

One more RFC... anyone?

-- 
Rafał
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Rafał Miłecki]

On 30 December 2015 at 12:10, Rafał Miłecki  wrote:
> Supported syntax is inspired by ethtool. Example usage:
> swconfig dev switch0 port 2 set link "duplex half speed 100 autoneg off"

Any comments to this usage/syntax? It doesn't look too nice (this long
quoted string as a value), but swconfig is strongly focused on simple
values. Making swconfig accept something like:
swconfig dev switch0 port 2 set link duplex half speed 100 autoneg off
would require big rewrite of handling arguments in main function.

Moreover swconfig uses magic mapping of UCI options into swconfig
attributes. Thanks to this - with proposed patch - I can simply use:
network.@switch_port[0]=switch_port
network.@switch_port[0].device='switch0'
network.@switch_port[0].port='3'
network.@switch_port[0].link='duplex full speed 10 autoneg on'
and it works without any extra changes.

Making swconfig understand more friendly options, e.g.
network.@switch_port[0].link_duplex
network.@switch_port[0].link_speed
network.@switch_port[0].link_autoneg
would require big changes to swlib_apply_from_uci and swlib_map_settings.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Stefan Rompf]

Hi Rafał,

> Supported syntax is inspired by ethtool. Example usage:
> swconfig dev switch0 port 2 set link "duplex half speed 100 autoneg off"
> 
> Signed-off-by: Rafał Miłecki 

I think that's the way to go, will try implementing set_port_link() for my 
good old 1043ND.

Ack for this and the kernel patches.

Stefan
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

[OpenWrt-Devel] [PATCH] swconfig: support setting SWITCH_TYPE_LINK attributes

[Rafał Miłecki]

Supported syntax is inspired by ethtool. Example usage:
swconfig dev switch0 port 2 set link "duplex half speed 100 autoneg off"

Signed-off-by: Rafał Miłecki 
---
 package/network/config/swconfig/src/swlib.c | 51 +
 1 file changed, 51 insertions(+)

diff --git a/package/network/config/swconfig/src/swlib.c 
b/package/network/config/swconfig/src/swlib.c
index 7bcd8d2..ddde452 100644
--- a/package/network/config/swconfig/src/swlib.c
+++ b/package/network/config/swconfig/src/swlib.c
@@ -430,11 +430,20 @@ swlib_set_attr(struct switch_dev *dev, struct switch_attr 
*attr, struct switch_v
return swlib_call(cmd, NULL, send_attr_val, val);
 }
 
+enum {
+   CMD_NONE,
+   CMD_DUPLEX,
+   CMD_ANEG,
+   CMD_SPEED,
+};
+
 int swlib_set_attr_string(struct switch_dev *dev, struct switch_attr *a, int 
port_vlan, const char *str)
 {
struct switch_port *ports;
+   struct switch_port_link *link;
struct switch_val val;
char *ptr;
+   int cmd = CMD_NONE;
 
memset(, 0, sizeof(val));
val.port_vlan = port_vlan;
@@ -480,6 +489,48 @@ int swlib_set_attr_string(struct switch_dev *dev, struct 
switch_attr *a, int por
}
val.value.ports = ports;
break;
+   case SWITCH_TYPE_LINK:
+   link = malloc(sizeof(struct switch_port_link));
+   memset(link, 0, sizeof(struct switch_port_link));
+   ptr = (char *)str;
+   for (ptr = strtok(ptr," "); ptr; ptr = strtok(NULL, " ")) {
+   switch (cmd) {
+   case CMD_NONE:
+   if (!strcmp(ptr, "duplex"))
+   cmd = CMD_DUPLEX;
+   else if (!strcmp(ptr, "autoneg"))
+   cmd = CMD_ANEG;
+   else if (!strcmp(ptr, "speed"))
+   cmd = CMD_SPEED;
+   else
+   fprintf(stderr, "Unsupported option 
%s\n", ptr);
+   break;
+   case CMD_DUPLEX:
+   if (!strcmp(ptr, "half"))
+   link->duplex = 0;
+   else if (!strcmp(ptr, "full"))
+   link->duplex = 1;
+   else
+   fprintf(stderr, "Unsupported value 
%s\n", ptr);
+   cmd = CMD_NONE;
+   break;
+   case CMD_ANEG:
+   if (!strcmp(ptr, "on"))
+   link->aneg = 1;
+   else if (!strcmp(ptr, "off"))
+   link->aneg = 0;
+   else
+   fprintf(stderr, "Unsupported value 
%s\n", ptr);
+   cmd = CMD_NONE;
+   break;
+   case CMD_SPEED:
+   link->speed = atoi(ptr);
+   cmd = CMD_NONE;
+   break;
+   }
+   }
+   val.value.link = link;
+   break;
case SWITCH_TYPE_NOVAL:
if (str && !strcmp(str, "0"))
return 0;
-- 
1.8.4.5
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel