This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
--
1.8.0
This allows a driver to register change_carrier callback which will be
called whenever user will like to change carrier state. This is useful
for devices like dummy, gre, team and so on.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
include/linux/netdevice.h | 7 +++++++
net/core/dev.c | 19 +++++++++++++++++++
2 files changed, 26 insertions(+)
From: Stephen Hemminger <hidden> Date: 2012-12-12 16:16:22
On Wed, 12 Dec 2012 11:58:03 +0100
Jiri Pirko [off-list ref] wrote:
This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
I needed to do the same thing for a project we are working on and discovered
that there already is a working documented interface for doing that via
operstate mode. Therefore I can't recommend that the additional complexity
of a new API for this is required.
Wed, Dec 12, 2012 at 05:15:00PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 11:58:03 +0100
Jiri Pirko [off-list ref] wrote:
quoted
This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
I needed to do the same thing for a project we are working on and discovered
that there already is a working documented interface for doing that via
operstate mode. Therefore I can't recommend that the additional complexity
of a new API for this is required.
I might be missing something, but I'm unable to find how operstate set
can affect value returned by netif_carrier_ok()
From: Stephen Hemminger <hidden> Date: 2012-12-12 17:28:18
On Wed, 12 Dec 2012 18:05:20 +0100
Jiri Pirko [off-list ref] wrote:
Wed, Dec 12, 2012 at 05:15:00PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 11:58:03 +0100
Jiri Pirko [off-list ref] wrote:
quoted
This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
I needed to do the same thing for a project we are working on and discovered
that there already is a working documented interface for doing that via
operstate mode. Therefore I can't recommend that the additional complexity
of a new API for this is required.
I might be missing something, but I'm unable to find how operstate set
can affect value returned by netif_carrier_ok()
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Here is an example using dummy device using libmnl. It is also possible
with ip commands.
# modprobe dummy
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 init
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ip li set dummy0 up
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 down
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 up
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
/* Sample program to control link mode and link state */
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <time.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/fcntl.h>
#include <sys/ioctl.h>
#include <libmnl/libmnl.h>
#include <linux/if.h>
#include <linux/if_tun.h>
#include <linux/rtnetlink.h>
static void panic(const char *str)
{
perror(str);
exit(1);
}
static void usage(const char *cmd)
{
fprintf(stderr, "Usage: %s dummyX [up|down|init]\n", cmd);
exit(1);
}
/* Send request and parse response */
static void mnl_talk(struct mnl_socket *nl, struct nlmsghdr *nlh)
{
unsigned portid = mnl_socket_get_portid(nl);
uint32_t seq = time(NULL);
char buf[MNL_SOCKET_BUFFER_SIZE];
nlh->nlmsg_flags |= NLM_F_ACK;
nlh->nlmsg_seq = seq;
if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0)
panic("mnl_socket_sendto failed");
int ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
if (ret < 0)
panic("mnl_socket_recvfrom");
if ( mnl_cb_run(buf, ret, seq, portid, NULL, NULL) < 0)
panic("mnl_cb_run");
}
static void linkstate(struct mnl_socket *nl,
const char *ifname, unsigned int state)
{
char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
nlh->nlmsg_type = RTM_NEWLINK;
nlh->nlmsg_flags = NLM_F_REQUEST;
struct ifinfomsg *ifi;
ifi = mnl_nlmsg_put_extra_header(nlh, sizeof(struct ifinfomsg));
ifi->ifi_family = AF_UNSPEC;
mnl_attr_put_strz(nlh, IFLA_IFNAME, ifname);
mnl_attr_put_u8(nlh, IFLA_OPERSTATE, state);
mnl_talk(nl, nlh);
}
/* Set device link mode */
static void init(struct mnl_socket *nl, const char *ifname)
{
char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
nlh->nlmsg_type = RTM_NEWLINK;
nlh->nlmsg_flags = NLM_F_REQUEST;
struct ifinfomsg *ifi;
ifi = mnl_nlmsg_put_extra_header(nlh, sizeof(struct ifinfomsg));
ifi->ifi_family = AF_UNSPEC;
mnl_attr_put_strz(nlh, IFLA_IFNAME, ifname);
mnl_attr_put_u8(nlh, IFLA_LINKMODE, IF_LINK_MODE_DORMANT);
mnl_talk(nl, nlh);
}
int main(int argc, char **argv)
{
if (argc != 3)
usage(argv[0]);
struct mnl_socket *nl = mnl_socket_open(NETLINK_ROUTE);
if (!nl)
panic("mnl_socket_open");
if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0)
panic("mnl_socket_bind");
if (strcmp(argv[2], "init") == 0)
init(nl, argv[1]);
else if (strcmp(argv[2], "up") == 0)
linkstate(nl, argv[1], IF_OPER_UP);
else if (strcmp(argv[2], "down") == 0)
linkstate(nl, argv[1], IF_OPER_DORMANT);
else
usage(argv[0]);
return 0;
}
Wed, Dec 12, 2012 at 06:27:00PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 18:05:20 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 05:15:00PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 11:58:03 +0100
Jiri Pirko [off-list ref] wrote:
quoted
This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
I needed to do the same thing for a project we are working on and discovered
that there already is a working documented interface for doing that via
operstate mode. Therefore I can't recommend that the additional complexity
of a new API for this is required.
I might be missing something, but I'm unable to find how operstate set
can affect value returned by netif_carrier_ok()
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Here is an example using dummy device using libmnl. It is also possible
with ip commands.
# modprobe dummy
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 init
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ip li set dummy0 up
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 down
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
# ./dummy dummy0 up
# ip li show dev dummy0
12: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DORMANT
link/ether ce:90:46:83:6e:f8 brd ff:ff:ff:ff:ff:ff
/* Sample program to control link mode and link state */
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <time.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/fcntl.h>
#include <sys/ioctl.h>
#include <libmnl/libmnl.h>
#include <linux/if.h>
#include <linux/if_tun.h>
#include <linux/rtnetlink.h>
static void panic(const char *str)
{
perror(str);
exit(1);
}
static void usage(const char *cmd)
{
fprintf(stderr, "Usage: %s dummyX [up|down|init]\n", cmd);
exit(1);
}
/* Send request and parse response */
static void mnl_talk(struct mnl_socket *nl, struct nlmsghdr *nlh)
{
unsigned portid = mnl_socket_get_portid(nl);
uint32_t seq = time(NULL);
char buf[MNL_SOCKET_BUFFER_SIZE];
nlh->nlmsg_flags |= NLM_F_ACK;
nlh->nlmsg_seq = seq;
if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0)
panic("mnl_socket_sendto failed");
int ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
if (ret < 0)
panic("mnl_socket_recvfrom");
if ( mnl_cb_run(buf, ret, seq, portid, NULL, NULL) < 0)
panic("mnl_cb_run");
}
static void linkstate(struct mnl_socket *nl,
const char *ifname, unsigned int state)
{
char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
nlh->nlmsg_type = RTM_NEWLINK;
nlh->nlmsg_flags = NLM_F_REQUEST;
struct ifinfomsg *ifi;
ifi = mnl_nlmsg_put_extra_header(nlh, sizeof(struct ifinfomsg));
ifi->ifi_family = AF_UNSPEC;
mnl_attr_put_strz(nlh, IFLA_IFNAME, ifname);
mnl_attr_put_u8(nlh, IFLA_OPERSTATE, state);
mnl_talk(nl, nlh);
}
/* Set device link mode */
static void init(struct mnl_socket *nl, const char *ifname)
{
char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
nlh->nlmsg_type = RTM_NEWLINK;
nlh->nlmsg_flags = NLM_F_REQUEST;
struct ifinfomsg *ifi;
ifi = mnl_nlmsg_put_extra_header(nlh, sizeof(struct ifinfomsg));
ifi->ifi_family = AF_UNSPEC;
mnl_attr_put_strz(nlh, IFLA_IFNAME, ifname);
mnl_attr_put_u8(nlh, IFLA_LINKMODE, IF_LINK_MODE_DORMANT);
mnl_talk(nl, nlh);
}
int main(int argc, char **argv)
{
if (argc != 3)
usage(argv[0]);
struct mnl_socket *nl = mnl_socket_open(NETLINK_ROUTE);
if (!nl)
panic("mnl_socket_open");
if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0)
panic("mnl_socket_bind");
if (strcmp(argv[2], "init") == 0)
init(nl, argv[1]);
else if (strcmp(argv[2], "up") == 0)
linkstate(nl, argv[1], IF_OPER_UP);
else if (strcmp(argv[2], "down") == 0)
linkstate(nl, argv[1], IF_OPER_DORMANT);
else
usage(argv[0]);
return 0;
}
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
From: Stephen Hemminger <hidden> Date: 2012-12-12 18:37:53
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
Wed, Dec 12, 2012 at 07:36:32PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
This patchset would provide a possibility to set or clear the carrier
from userspace. For dummy device it would serve for direct emulation
of link fail.
Also for team deriver, that would serve for teamd (userspace part) to
set the carrier actually on or off (in case of LACP runner for example
this is required).
From: Stephen Hemminger <hidden> Date: 2012-12-12 18:56:04
On Wed, 12 Dec 2012 19:49:26 +0100
Jiri Pirko [off-list ref] wrote:
Wed, Dec 12, 2012 at 07:36:32PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
This patchset would provide a possibility to set or clear the carrier
from userspace. For dummy device it would serve for direct emulation
of link fail.
Also for team deriver, that would serve for teamd (userspace part) to
set the carrier actually on or off (in case of LACP runner for example
this is required).
You want to able to control the dummy device, so that you can test carrier
management in the team device. Another alternative is to use carrier control
on a virtual device. Vmware can do it, there were patches to do this with KVM/QEMU
not sure if they ever got incorporated.
Since this is a specific feature of the dummy device which is specialized for
testing, maybe it should just be done by adding device specific ioctl rather
than letting it creep in as a general facility.
Wed, Dec 12, 2012 at 07:54:48PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 19:49:26 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:36:32PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
This patchset would provide a possibility to set or clear the carrier
from userspace. For dummy device it would serve for direct emulation
of link fail.
Also for team deriver, that would serve for teamd (userspace part) to
set the carrier actually on or off (in case of LACP runner for example
this is required).
You want to able to control the dummy device, so that you can test carrier
management in the team device. Another alternative is to use carrier control
on a virtual device. Vmware can do it, there were patches to do this with KVM/QEMU
not sure if they ever got incorporated.
Since this is a specific feature of the dummy device which is specialized for
testing, maybe it should just be done by adding device specific ioctl rather
than letting it creep in as a general facility.
Ugh, specific ioctl stinks...
But this is not only for dummy. As I said, we need this for team driver.
Maybe I did not explain that correctly. Given the fact that the whole
Team logic is in userspace, teamd (userspace daemon) needs to set the
carrier state as if it was done in kernel. Yes, we would be able to do
this by specific Team option in team driver, but I thought this would be
nicer to do that more generally.
Also, in previous discussion Michał Mirosław wrote he would like this
feature also for GRE tunnel devices.
From: Stephen Hemminger <hidden> Date: 2012-12-12 19:35:50
On Wed, 12 Dec 2012 20:06:13 +0100
Jiri Pirko [off-list ref] wrote:
Wed, Dec 12, 2012 at 07:54:48PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:49:26 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:36:32PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
This patchset would provide a possibility to set or clear the carrier
from userspace. For dummy device it would serve for direct emulation
of link fail.
Also for team deriver, that would serve for teamd (userspace part) to
set the carrier actually on or off (in case of LACP runner for example
this is required).
You want to able to control the dummy device, so that you can test carrier
management in the team device. Another alternative is to use carrier control
on a virtual device. Vmware can do it, there were patches to do this with KVM/QEMU
not sure if they ever got incorporated.
Since this is a specific feature of the dummy device which is specialized for
testing, maybe it should just be done by adding device specific ioctl rather
than letting it creep in as a general facility.
Ugh, specific ioctl stinks...
But this is not only for dummy. As I said, we need this for team driver.
Maybe I did not explain that correctly. Given the fact that the whole
Team logic is in userspace, teamd (userspace daemon) needs to set the
carrier state as if it was done in kernel. Yes, we would be able to do
this by specific Team option in team driver, but I thought this would be
nicer to do that more generally.
That is what the operstate mechanism was for. Why did we build that mechanism
if it doesn't work from userspace.
Maybe the fix is to make setting linkstate also set carrier bits.
Wed, Dec 12, 2012 at 08:34:33PM CET, shemminger@vyatta.com wrote:
On Wed, 12 Dec 2012 20:06:13 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:54:48PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:49:26 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:36:32PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:25:56 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Wed, Dec 12, 2012 at 07:12:08PM CET, shemminger@vyatta.com wrote:
quoted
On Wed, 12 Dec 2012 19:10:17 +0100
Jiri Pirko [off-list ref] wrote:
quoted
quoted
# ip li show dev dummy0
12: dummy0: <NO-CARRIER,BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state DORMANT mode DORMANT
if you mean this "NO-CARRIER"
it has no direct relation with netif_carrier_ok().
It is the same value (IFF_RUNNING) that is visible from user space.
static inline bool netif_carrier_ok(const struct net_device *dev)
{
return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
}
So netif_carrier[ok/on/off] are working with on __LINK_STATE_NOCARRIER
bit. Not with IFF_RUNNING flag.
What is the code path that you are worried about netif_carrier_ok being set or clear?
The interaction here is complex, and right now LINK_STATE_NOCARRIER is purely
controlled by the driver, your patch changes that, but before acking I want
to make sure why it is required.
This patchset would provide a possibility to set or clear the carrier
from userspace. For dummy device it would serve for direct emulation
of link fail.
Also for team deriver, that would serve for teamd (userspace part) to
set the carrier actually on or off (in case of LACP runner for example
this is required).
You want to able to control the dummy device, so that you can test carrier
management in the team device. Another alternative is to use carrier control
on a virtual device. Vmware can do it, there were patches to do this with KVM/QEMU
not sure if they ever got incorporated.
Since this is a specific feature of the dummy device which is specialized for
testing, maybe it should just be done by adding device specific ioctl rather
than letting it creep in as a general facility.
Ugh, specific ioctl stinks...
But this is not only for dummy. As I said, we need this for team driver.
Maybe I did not explain that correctly. Given the fact that the whole
Team logic is in userspace, teamd (userspace daemon) needs to set the
carrier state as if it was done in kernel. Yes, we would be able to do
this by specific Team option in team driver, but I thought this would be
nicer to do that more generally.
That is what the operstate mechanism was for. Why did we build that mechanism
if it doesn't work from userspace.
Maybe the fix is to make setting linkstate also set carrier bits.
Hmm. You mean to call netif_carrier_on/off as a reaction to operstate
change? How exactly would you like to do that?
Thanks
Jiri
From: John Fastabend <john.fastabend@gmail.com> Date: 2012-12-13 17:16:37
[...]
quoted
That is what the operstate mechanism was for. Why did we build that mechanism
if it doesn't work from userspace.
Maybe the fix is to make setting linkstate also set carrier bits.
Hmm. You mean to call netif_carrier_on/off as a reaction to operstate
change? How exactly would you like to do that?
Thanks
Jiri
This would break existing applications and would not really be in the
spirit of the operstate mechanism as I read the documentation:
./Documentation/networking/operstates.txt
63 IF_OPER_DORMANT (5):
64 Interface is L1 up, but waiting for an external event, f.e. for a
65 protocol to establish. (802.1X)
The L1 up is netif_carrier_on here.
We use this in user space when we do not want applications to start
using the link until we have negotiated and configured some link layer
attributes. To do this we set IFLA_LINKMODE and then use the
IF_OPER_DORMANT event to trigger the application eventually setting
IF_OPER_UP when the link layer negotiation is complete. If you take the
carrier down this breaks. In my case the protocol is LLDP but I think
there are other examples. This is basically the example Stephen already
gave.
I guess I still am missing why teamd doesn't just set IFLA_LINKMODE
then manage the operstate this way? Sure teamd would have to become a
bit smarter but it would save adding additional interfaces to the
kernel. In the LinkAgg case you could just pin the operstate down this
would also allow protocols to run under the linkagg over the LLC in
IEEE speak which I think is being discussed in the latest round of
LLDP/LinkAgg spec updates (I'll check on that later today).
Thanks,
John
--
John Fastabend Intel Corporation
On Thu, 13 Dec 2012 09:15:41 -0800
John Fastabend [off-list ref] wrote:
[...]
quoted
quoted
That is what the operstate mechanism was for. Why did we build that mechanism
if it doesn't work from userspace.
Maybe the fix is to make setting linkstate also set carrier bits.
Hmm. You mean to call netif_carrier_on/off as a reaction to operstate
change? How exactly would you like to do that?
Thanks
Jiri
This would break existing applications and would not really be in the
spirit of the operstate mechanism as I read the documentation:
./Documentation/networking/operstates.txt
63 IF_OPER_DORMANT (5):
64 Interface is L1 up, but waiting for an external event, f.e. for a
65 protocol to establish. (802.1X)
The L1 up is netif_carrier_on here.
Agreed. I am also not finding how to change carrier bits from the
current states.
We use this in user space when we do not want applications to start
using the link until we have negotiated and configured some link layer
attributes. To do this we set IFLA_LINKMODE and then use the
IF_OPER_DORMANT event to trigger the application eventually setting
IF_OPER_UP when the link layer negotiation is complete. If you take the
carrier down this breaks. In my case the protocol is LLDP but I think
there are other examples. This is basically the example Stephen already
gave.
I guess I still am missing why teamd doesn't just set IFLA_LINKMODE
then manage the operstate this way? Sure teamd would have to become a
bit smarter but it would save adding additional interfaces to the
kernel. In the LinkAgg case you could just pin the operstate down this
would also allow protocols to run under the linkagg over the LLC in
IEEE speak which I think is being discussed in the latest round of
LLDP/LinkAgg spec updates (I'll check on that later today).
Think about the ARP monitoring as an example. If the teamd (userlevel)
doesn't receive the reply packet, it should set the master interface
as down in terms of carrier because there isn't a valid L1 from the
master interface perspective.
LACP needs the same action. If the ports can't move to DISTRIBUTING/
COLLECTING state, it should set master's carrier bit to down.
Even the lack of ports should keep the master with carrier down.
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Therefore, I am not seeing how setting operstate to down helps with the
current code. Can ethtool report Link based on a logic between operstate
and netif_carrier_ok()? If operstate is down, it would report down regardless
of netif_carrier_ok() for instance.
Calling netif_carrier_off() also calls dev_deactivate() to stop everything.
So, if team sets operstate to down, the interface remains active sending/
receiving as far as I can see, right? Not sure if this would be a problem.
--
fbl
From: Stephen Hemminger <hidden> Date: 2012-12-13 18:10:52
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
--
fbl
From: Stephen Hemminger <hidden> Date: 2012-12-13 18:22:06
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
From: Dan Williams <hidden> Date: 2012-12-13 18:32:28
On Thu, 2012-12-13 at 10:20 -0800, Stephen Hemminger wrote:
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
That's not entirely sufficient, because not everything uses ethtool to
deterine whether the link/carrier is active. eg, anything listenting to
netlink for IFF_RUNNING won't know anything about this. I may have
missed previous conversation, but IFF_RUNNING is AFAIK the sole
mechanism to indicate to userspace that the link is operational and
ready for general traffic. Can the team drivers simply not set
IFF_RUNNING until the interface is actually ready for general traffic?
I'd just caution people to be careful when it comes to carrier/link
states, as a lot of stuff relies on it, and people keep trying to
slightly change the meaning of it. We need it to be consistent across
net interfaces and driver implementations, not have specific meanings to
certain drivers that don't apply to others.
If teamd needs to manage the carrier state of the teamX interface,
perhaps instead of munging the actual carrier state itself, it can flip
some team-private value that is one component of determining whether
IFF_RUNNING gets set or not?
Dan
From: John Fastabend <hidden> Date: 2012-12-13 19:09:07
On 12/13/2012 10:33 AM, Dan Williams wrote:
On Thu, 2012-12-13 at 10:20 -0800, Stephen Hemminger wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
That's not entirely sufficient, because not everything uses ethtool to
deterine whether the link/carrier is active. eg, anything listenting to
netlink for IFF_RUNNING won't know anything about this. I may have
missed previous conversation, but IFF_RUNNING is AFAIK the sole
mechanism to indicate to userspace that the link is operational and
ready for general traffic. Can the team drivers simply not set
IFF_RUNNING until the interface is actually ready for general traffic?
I'd just caution people to be careful when it comes to carrier/link
states, as a lot of stuff relies on it, and people keep trying to
slightly change the meaning of it. We need it to be consistent across
net interfaces and driver implementations, not have specific meanings to
certain drivers that don't apply to others.
If teamd needs to manage the carrier state of the teamX interface,
perhaps instead of munging the actual carrier state itself, it can flip
some team-private value that is one component of determining whether
IFF_RUNNING gets set or not?
Dan
Assuming the operstate documentation is correct IFF_RUNNING should only
be set if the operstate is up or unknown. Quoting the text again,
32 ifinfomsg::if_flags & IFF_RUNNING:
33 Interface is in RFC2863 operational state UP or UNKNOWN. This is for
34 backward compatibility, routing daemons, dhcp clients can use this
35 flag to determine whether they should use the interface.
So my guess is having a team ethtool get_link handler AND managing the
operstate from teamd would satisfy the requirement. But maybe I missed
something.
.John
Thu, Dec 13, 2012 at 08:09:03PM CET, john.r.fastabend@intel.com wrote:
On 12/13/2012 10:33 AM, Dan Williams wrote:
quoted
On Thu, 2012-12-13 at 10:20 -0800, Stephen Hemminger wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
That's not entirely sufficient, because not everything uses ethtool to
deterine whether the link/carrier is active. eg, anything listenting to
netlink for IFF_RUNNING won't know anything about this. I may have
missed previous conversation, but IFF_RUNNING is AFAIK the sole
mechanism to indicate to userspace that the link is operational and
ready for general traffic. Can the team drivers simply not set
IFF_RUNNING until the interface is actually ready for general traffic?
I'd just caution people to be careful when it comes to carrier/link
states, as a lot of stuff relies on it, and people keep trying to
slightly change the meaning of it. We need it to be consistent across
net interfaces and driver implementations, not have specific meanings to
certain drivers that don't apply to others.
If teamd needs to manage the carrier state of the teamX interface,
perhaps instead of munging the actual carrier state itself, it can flip
some team-private value that is one component of determining whether
IFF_RUNNING gets set or not?
Dan
Assuming the operstate documentation is correct IFF_RUNNING should only
be set if the operstate is up or unknown. Quoting the text again,
32 ifinfomsg::if_flags & IFF_RUNNING:
33 Interface is in RFC2863 operational state UP or UNKNOWN. This is for
34 backward compatibility, routing daemons, dhcp clients can use this
35 flag to determine whether they should use the interface.
So my guess is having a team ethtool get_link handler AND managing the
operstate from teamd would satisfy the requirement. But maybe I missed
something.
Yes, I've been thinking about the same thing. I'll check that.
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
From: Stephen Hemminger <hidden> Date: 2012-12-14 16:13:17
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
From: Stephen Hemminger <hidden> Date: 2012-12-14 17:00:34
On Fri, 14 Dec 2012 17:35:32 +0100
Jiri Pirko [off-list ref] wrote:
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
Fri, Dec 14, 2012 at 05:59:18PM CET, shemminger@vyatta.com wrote:
quoted hunk
On Fri, 14 Dec 2012 17:35:32 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
Yes. I have this already in my queue. I just spotted a problem though.
Lets say teamd sets operstate of the team device by values IF_OPER_UP
and IF_OPER_DORMANT depending on teamd states of ports.
What if one would like to use 802.1X supplicant on the same device?
That would not be possible.
This proves that the layering would not be correct. It look like the
carrier userspace set would be the correct thing to do after all...
What do you think?
Jiri
From: Stephen Hemminger <hidden> Date: 2012-12-14 17:24:59
On Fri, 14 Dec 2012 18:13:45 +0100
Jiri Pirko [off-list ref] wrote:
Fri, Dec 14, 2012 at 05:59:18PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 17:35:32 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
Yes. I have this already in my queue. I just spotted a problem though.
Lets say teamd sets operstate of the team device by values IF_OPER_UP
and IF_OPER_DORMANT depending on teamd states of ports.
What if one would like to use 802.1X supplicant on the same device?
That would not be possible.
This proves that the layering would not be correct. It look like the
carrier userspace set would be the correct thing to do after all...
What do you think?
That is tough, you have two applications conflicting over control of
the same state on the same device.
Fri, Dec 14, 2012 at 06:23:43PM CET, shemminger@vyatta.com wrote:
On Fri, 14 Dec 2012 18:13:45 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Fri, Dec 14, 2012 at 05:59:18PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 17:35:32 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
quoted
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko [off-list ref] wrote:
quoted
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
quoted
On Thu, 13 Dec 2012 16:17:33 -0200
Flavio Leitner [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 10:09:33 -0800
Stephen Hemminger [off-list ref] wrote:
quoted
On Thu, 13 Dec 2012 15:54:23 -0200
Flavio Leitner [off-list ref] wrote:
quoted
I am saying this because people are used to and there are scripts out
there using something like:
# ethtool <iface> | grep 'Link'
to react an interface failure.
Then the script is broken. It is asking about hardware state.
I was talking about the team master interface, so it makes sense
to check its 'hardware' state. Just think on 'bond0' interface
with no slaves. It should report Link detected: no.
See bond_release(), what happens if bond->slave_cnt == 0, for instance.
I was thinking more that ethtool operation for reporting link on
the team device should use the proper check rather than just using netif_carrier_ok(),
the team ethtool operation for get_link should be check IFF_RUNNING flag
in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
Yes. I have this already in my queue. I just spotted a problem though.
Lets say teamd sets operstate of the team device by values IF_OPER_UP
and IF_OPER_DORMANT depending on teamd states of ports.
What if one would like to use 802.1X supplicant on the same device?
That would not be possible.
This proves that the layering would not be correct. It look like the
carrier userspace set would be the correct thing to do after all...
What do you think?
That is tough, you have two applications conflicting over control of
the same state on the same device.
I do not think so. The thing is, they both do something different. Teamd
should care of the low level link and set that up accordingly.
802.1X supplicant is higher. This implies they should not use the same
interface, ergo IMHO Teamd should be able to set carrier directly.
And that is what is provided by this patchset.
ps: I think that the use of 802.1X supplicant over team device is legit
setup.
I see that the patchset is in state "Rejected" in patchwork.
Stephen convinced me for a moment that the problem can be handled by operstate.
As it turned out (in last 3-4 emails in thread) operstate use would not
be an option.
So how should I proceed? Should I repost the patchset? Anyone has any other
comments?
thanks.
Wed, Dec 12, 2012 at 11:58:03AM CET, jiri@resnulli.us wrote:
This is basically a repost of my previous patchset:
"[patch net-next-2.6 0/2] net: allow to change carrier via sysfs" from Aug 30
The way net-sysfs stores values changed and this patchset reflects it.
Also, I exposed carrier via rtnetlink iface.
So far, only dummy driver uses carrier change ndo. In very near future
team driver will use that as well.
Jiri Pirko (4):
net: add change_carrier netdev op
net: allow to change carrier via sysfs
rtnl: expose carrier value with possibility to set it
dummy: implement carrier change
drivers/net/dummy.c | 10 ++++++++++
include/linux/netdevice.h | 7 +++++++
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
net/core/net-sysfs.c | 15 ++++++++++++++-
net/core/rtnetlink.c | 10 ++++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
--
1.8.0
From: Stephen Hemminger <hidden> Date: 2012-12-18 06:51:16
On Sun, 16 Dec 2012 11:54:51 +0100
Jiri Pirko [off-list ref] wrote:
I see that the patchset is in state "Rejected" in patchwork.
Stephen convinced me for a moment that the problem can be handled by operstate.
As it turned out (in last 3-4 emails in thread) operstate use would not
be an option.
So how should I proceed? Should I repost the patchset? Anyone has any other
comments?
thanks.
Don't take my comments so far as negative. Devices to need to be more controllable
from userspace. But I have concerns about introducing a new way to change state causing
more races. For example, changing carrier state should cause netlink events to fire and
these should post to routing daemons etc. Also, what happens if some confused developer
mixes operstate and direct carrier control.
The root cause of all this confusion is that their are three ways of expressing
the same state, and they are controlled through different paths:
a. Old BSD style flag bit IFF_RUNNING
b. LINK_STATE bit in kernel (netif_carrier_ok)
c. RFC2863 operational state
The operstate stuff is the most complete, but is the weakest in implementation:
a. kernel drivers check netif_carrier_ok when they should be using netif_dormant
(bridge is one example). But what will break if this changes?
b. lower device state is not tracked correctly by tunnels and a few other layered devices
c. dormant from kernel space was never used by much.
The good news is that the old BSD style IFF_RUNNING bit is the most commonly
used bit by applications and it works correctly in either carrier or operstate mode.
Tue, Dec 18, 2012 at 07:49:57AM CET, shemminger@vyatta.com wrote:
On Sun, 16 Dec 2012 11:54:51 +0100
Jiri Pirko [off-list ref] wrote:
quoted
I see that the patchset is in state "Rejected" in patchwork.
Stephen convinced me for a moment that the problem can be handled by operstate.
As it turned out (in last 3-4 emails in thread) operstate use would not
be an option.
So how should I proceed? Should I repost the patchset? Anyone has any other
comments?
thanks.
Don't take my comments so far as negative. Devices to need to be more controllable
from userspace. But I have concerns about introducing a new way to change state causing
more races. For example, changing carrier state should cause netlink events to fire and
these should post to routing daemons etc. Also, what happens if some confused developer
mixes operstate and direct carrier control.
I do not think that the race you are describing is of any concern. The
same can happen now for any device. My patchset only adds a possibility
for "soft devices" to change the carrier as well.
Developer will not be likely confused. As the possibility of carrier
change from userspace will be limited to small set of devices, for other
devices the attempt will lead to -EOPNOTSUPP (in contrast with operstate
which is available for all devices).
I can add a comments/notes to code and operstates.txt stating the
purpose of this iface.
The root cause of all this confusion is that their are three ways of expressing
the same state, and they are controlled through different paths:
a. Old BSD style flag bit IFF_RUNNING
b. LINK_STATE bit in kernel (netif_carrier_ok)
c. RFC2863 operational state
I do not think so. Yes, for a) and c), these are strictly connected,
expressing the same thing. But b) is not the same. It's on lower level
than a) and c). What b) can be compared to is IFF_LOWER_UP.
The operstate stuff is the most complete, but is the weakest in implementation:
a. kernel drivers check netif_carrier_ok when they should be using netif_dormant
(bridge is one example). But what will break if this changes?
I agree, that should be changed.
b. lower device state is not tracked correctly by tunnels and a few other layered devices
c. dormant from kernel space was never used by much.
The good news is that the old BSD style IFF_RUNNING bit is the most commonly
used bit by applications and it works correctly in either carrier or operstate mode.