Message ID | 1474966541-4420-5-git-send-email-makita.toshiaki@lab.ntt.co.jp |
---|---|
State | RFC, archived |
Delegated to: | stephen hemminger |
Headers | show |
On Tue, 27 Sep 2016 17:55:40 +0900, Toshiaki Makita wrote: > This adds support for envhdrlen. > > Example: > # ip link set eno1 envhdrlen 8 I don't see why this should be user visible, let alone requiring user to set it. This should be transparent, kernel should compute the value as needed based on the configuration and set it up. Requiring the administrator to pick up a calculator and sum up all the vlan, mpls and whatever header lengths is silly. I realize that we currently have no easy way to do that. Especially with lwtunnels and stuff line MPLS where we don't easily know the number of tags. But every uAPI we introduce will have to be supported forever and going a particular way just because it is easy to implement is not sustainable. At the very least, it should be configurable from the other direction. I.e. telling which interfaces can be used by vlans or MPLS (if it cannot be inferred automatically) and configuring maximum number of tags on the given vlan/mpls/whatever interface/route/whatever. Jiri
On 16/09/29 (木) 21:49, Jiri Benc wrote: > On Tue, 27 Sep 2016 17:55:40 +0900, Toshiaki Makita wrote: >> This adds support for envhdrlen. >> >> Example: >> # ip link set eno1 envhdrlen 8 Thank you for taking a look at this. > I don't see why this should be user visible, let alone requiring user > to set it. This should be transparent, kernel should compute the value > as needed based on the configuration and set it up. Requiring the > administrator to pick up a calculator and sum up all the vlan, mpls and > whatever header lengths is silly. I'm thinking both in-kernel automation (for VLAN) and users' manual operation are necessary. In fact, as I stated in the cover letter, my first implementation was an automation-based approach. Actually automation is advanced form of this feature so I'm proposing this very basic feature first. > I realize that we currently have no easy way to do that. Especially > with lwtunnels and stuff line MPLS where we don't easily know the > number of tags. But every uAPI we introduce will have to be supported > forever and going a particular way just because it is easy to implement > is not sustainable. It may be possible for MPLS lwtunnel to notify underlying device of needed header length. But at this point MPLS does not allow encapsulated packets to be greater than MTU of underlying device. So how to determine if someone wants to leverage envhdrlen instead? Add a knob to lwtunnel layer? Then, add another knob to l2tp, iptunnel, or anything like that? At least full-automation does not look possible other than VLAN (which by default requires envhdrlen expansion), so anyway manual operation is needed in some form. Another use-case is reducing envhdrlen. We can expand it on creating VLAN device automatically, but cannot decrease the size because there could be other consumers of envhdrlen. > At the very least, it should be configurable from the other direction. > I.e. telling which interfaces can be used by vlans or MPLS (if it > cannot be inferred automatically) and configuring maximum number of > tags on the given vlan/mpls/whatever interface/route/whatever. Probably I don't get your point... Are you suggesting something like this? $ ip link set eth0.10.20 expand-realdev-envhdrlen or like this? $ ip link set eth0 allowed-vlan-tags 2 Toshiaki Makita
diff --git a/include/linux/if_link.h b/include/linux/if_link.h index b9299e3..46ef8cc 100644 --- a/include/linux/if_link.h +++ b/include/linux/if_link.h @@ -157,6 +157,7 @@ enum { IFLA_GSO_MAX_SIZE, IFLA_PAD, IFLA_XDP, + IFLA_ENV_HDR_LEN, __IFLA_MAX }; diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 76bd7b3..92a472d 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -820,6 +820,8 @@ int print_linkinfo(const struct sockaddr_nl *who, if (tb[IFLA_MTU]) fprintf(fp, "mtu %u ", *(int *)RTA_DATA(tb[IFLA_MTU])); + if (tb[IFLA_ENV_HDR_LEN]) + fprintf(fp, "envhdrlen %u ", *(int *)RTA_DATA(tb[IFLA_ENV_HDR_LEN])); if (tb[IFLA_QDISC]) fprintf(fp, "qdisc %s ", rta_getattr_str(tb[IFLA_QDISC])); if (tb[IFLA_MASTER]) { diff --git a/ip/iplink.c b/ip/iplink.c index 6b1db18..4dcb9ac 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -50,6 +50,7 @@ void iplink_usage(void) fprintf(stderr, " [ address LLADDR ]\n"); fprintf(stderr, " [ broadcast LLADDR ]\n"); fprintf(stderr, " [ mtu MTU ] [index IDX ]\n"); + fprintf(stderr, " [ envhdrlen ENVHDRLEN ]\n"); fprintf(stderr, " [ numtxqueues QUEUE_COUNT ]\n"); fprintf(stderr, " [ numrxqueues QUEUE_COUNT ]\n"); fprintf(stderr, " type TYPE [ ARGS ]\n"); @@ -489,6 +490,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char abuf[32]; int qlen = -1; int mtu = -1; + int envhdrlen = -1; int netns = -1; int vf = -1; int numtxqueues = -1; @@ -547,6 +549,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, if (get_integer(&mtu, *argv, 0)) invarg("Invalid \"mtu\" value\n", *argv); addattr_l(&req->n, sizeof(*req), IFLA_MTU, &mtu, 4); + } else if (strcmp(*argv, "envhdrlen") == 0) { + NEXT_ARG(); + if (envhdrlen != -1) + duparg("envhdrlen", *argv); + if (get_integer(&envhdrlen, *argv, 0)) + invarg("Invalid \"envhdrlen\" value\n", *argv); + addattr_l(&req->n, sizeof(*req), IFLA_ENV_HDR_LEN, + &envhdrlen, 4); } else if (strcmp(*argv, "netns") == 0) { NEXT_ARG(); if (netns != -1)
This adds support for envhdrlen. Example: # ip link set eno1 envhdrlen 8 # ip link show eno1 2: eno1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 envhdrlen 8 qdisc fq_codel state UP mode DEFAULT group default qlen 1000 link/ether 44:37:e6:6c:69:a4 brd ff:ff:ff:ff:ff:ff Note: As an RFC, this includes update for kernel headers. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- include/linux/if_link.h | 1 + ip/ipaddress.c | 2 ++ ip/iplink.c | 10 ++++++++++ 3 files changed, 13 insertions(+)