Message ID | 1451473815-23384-1-git-send-email-zajec5@gmail.com |
---|---|
State | Accepted |
Delegated to: | Rafał Miłecki |
Headers | show |
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 <zajec5@gmail.com> 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
On 30 December 2015 at 12:10, Rafał Miłecki <zajec5@gmail.com> 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.
On 7 January 2016 at 07:53, Rafał Miłecki <zajec5@gmail.com> wrote: > On 30 December 2015 at 12:10, Rafał Miłecki <zajec5@gmail.com> 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?
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
On Jan 13, 2016 10:07 PM, "Stefan Rompf" <stefan@loplof.de> 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
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(&val, 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;
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 <zajec5@gmail.com> --- package/network/config/swconfig/src/swlib.c | 51 +++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)