* [PATCH RFC v1 0/2] ethtool: add support for VF in ring_cookie @ 2016-12-06 12:40 Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 1/2] rxclass: use ethtool_get_flow_spec_ring to display queue and VF Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 2/2] ethtool: add support to specify ring_cookie as VF/queue Jacob Keller 0 siblings, 2 replies; 3+ messages in thread From: Jacob Keller @ 2016-12-06 12:40 UTC (permalink / raw) To: John W . Linville; +Cc: netdev, Jacob Keller The ring_cookie value for an ntuple filter is used by the driver to determine what queue to direct the traffic towards. Since the main function has cnotrol over all queues it is possible to use this to direct traffic to a queue on a virtual function rather than a queue on the physical function. However, this was cumbersome to do if you did not know exactly how the device laid out virtual function queues. To alleviate this, the kernel side of the ethtool API gained a couple of functions used to partition the ring_cookie value into a queue index, and a virtual function index. It was assumed that no device was likely to have more than 2^32 queues, and currently no device has more than 2^8 virtual functions, so the split was chosen such that the lower 32 bits of the ring_cookie represent the queue, and the next 8 bits would represent a virtual function id. This is useful, but lacked support on the ethtool command line side. It is possible to simply use hex notation, and to "know" that the driver works in this manner. However, this makes use of and knowledge of the feature not widespread. This series attempts to fix that with two patches. First, we make use of the access functions when displaying the queue. Since no device currently supports queues beyond 2^32 it is unlikely that any driver which does not honor this partitioning of the ring_cookie would have its index incorrectly interpreted. This makes it so that the user will see the queue index and virtual function index split apart instead of being displayed as a combined base-10 number. Additionally, the second patch adds support for a new notation for the action. Instead of just using a number, we add support for the special syntax "X/Y" where X would be a number from 0-256 which is the virtual function index, while Y would be a number from 0 to 2^32 which is the queue index. This makes it much easier to write the encoded ring_cookie value. This series is sent as an RFC so that others may comment on the design of the new syntax, and offer alternative suggestions. Some of the ones I have considered: a) just leave the second part alone, and display the VF+queue split, but do not provide an easier syntax for writing the split value. This is easy to do, but leaves the feature as a sort of hidden gem. Additionally, the user would most easily do this by writing it as hex notation such as action 0x100000005 However, this is cumbersome. It can be partially alleviated by using shell scripting on top of ethtool, but again is somewhat of a pain. b) actually adding a new key work so that you would do something like "queue x" and "vf y" or similar. However, this is much more difficult to correctly implement. The current implementation of handling the values assumes that each type argument is followed by a single argument which fits into a well defined section of the structure. Because the structure is not specified as le64 or be64, we can't use a union or similar to make it correct (since placement in the union ordering would depend on byte ordering of the field!) so we would have to re-architect the handling of values to allow multiple keywords to combine into a single value. This seemed incredibly cumbersome. The disadvantage of the new syntax is that it *is* new syntax that we have to parse ourselves, and that may not be obvious to users. However, we can document it, and the current syntax will work for all drivers, and the new syntax will generally only work on drivers which use the ring_cookie partitioning in this manner (as no current driver has queues beyond 2^32, as was decided when the partitioning scheme was added to the ethtool header file). I like the use of / but am open to alternative suggestions for a simple sequence. Note that we cannot use spaces, as this would move into the argv solution of (b) which I do not have time to implement, and would be a much more massive project. Jacob Keller (2): rxclass: use ethtool_get_flow_spec_ring to display queue and VF ethtool: add support to specify ring_cookie as VF/queue ethtool.8.in | 3 ++- rxclass.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 12 deletions(-) -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH RFC v1 1/2] rxclass: use ethtool_get_flow_spec_ring to display queue and VF 2016-12-06 12:40 [PATCH RFC v1 0/2] ethtool: add support for VF in ring_cookie Jacob Keller @ 2016-12-06 12:40 ` Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 2/2] ethtool: add support to specify ring_cookie as VF/queue Jacob Keller 1 sibling, 0 replies; 3+ messages in thread From: Jacob Keller @ 2016-12-06 12:40 UTC (permalink / raw) To: John W . Linville; +Cc: netdev, Jacob Keller Recent kernels have made the ring_cookie store both the Queue index as well as a VF identifier. This allows for drivers to direct traffic to specific VF queues, without having the user have to understand the physical queue layout. Add support to display this notation so that users do not have to manually parse the value. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- rxclass.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/rxclass.c b/rxclass.c index c7bfebaf6e22..7f0e765d3b47 100644 --- a/rxclass.c +++ b/rxclass.c @@ -247,11 +247,19 @@ static void rxclass_print_nfc_rule(struct ethtool_rx_flow_spec *fsp) rxclass_print_nfc_spec_ext(fsp); - if (fsp->ring_cookie != RX_CLS_FLOW_DISC) - fprintf(stdout, "\tAction: Direct to queue %llu\n", - fsp->ring_cookie); - else + if (fsp->ring_cookie != RX_CLS_FLOW_DISC) { + u64 vf = ethtool_get_flow_spec_ring_vf(fsp->ring_cookie); + u64 queue = ethtool_get_flow_spec_ring(fsp->ring_cookie); + + if (vf) + fprintf(stdout, "\tAction: Direct to queue %llu\n", + queue); + else + fprintf(stdout, "\tAction: Direct to VF %llu queue %llu\n", + vf, queue); + } else { fprintf(stdout, "\tAction: Drop\n"); + } fprintf(stdout, "\n"); } -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH RFC v1 2/2] ethtool: add support to specify ring_cookie as VF/queue 2016-12-06 12:40 [PATCH RFC v1 0/2] ethtool: add support for VF in ring_cookie Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 1/2] rxclass: use ethtool_get_flow_spec_ring to display queue and VF Jacob Keller @ 2016-12-06 12:40 ` Jacob Keller 1 sibling, 0 replies; 3+ messages in thread From: Jacob Keller @ 2016-12-06 12:40 UTC (permalink / raw) To: John W . Linville; +Cc: netdev, Jacob Keller Add support to allow users to specify the ring_cookie as a VF/queue value, ie: '0/25' means direct to the physical function queue 25, while '3/10' means to direct to VF id 3, queue 10. It is driver dependent to determine what the VF id is, excepting that 0 indicates the physical function. This allows the user to more easily specify the matching notation as it is understood by the kernel and some drivers. Since no driver exists today with over 2,147,483,647 queues, this notation will simply fail on drivers which don't recognize the new partitioning. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- ethtool.8.in | 3 ++- rxclass.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/ethtool.8.in b/ethtool.8.in index 5c36c06385f6..cb165fa4c77a 100644 --- a/ethtool.8.in +++ b/ethtool.8.in @@ -826,7 +826,8 @@ Specifies the Rx queue to send packets to, or some other action. nokeep; lB l. -1 Drop the matched flow -0 or higher Rx queue to route the flow +0 or higher Route flow to specific Rx queue on the physical function +N/M Route flow to Rx queue M of specific virtual function N (0 indicates the physical function) .TE .TP .BI loc \ N diff --git a/rxclass.c b/rxclass.c index 7f0e765d3b47..de25550529b4 100644 --- a/rxclass.c +++ b/rxclass.c @@ -614,6 +614,7 @@ typedef enum { OPT_IP4, OPT_IP6, OPT_MAC, + OPT_ACTION, } rule_opt_type_t; #define NFC_FLAG_RING 0x001 @@ -654,7 +655,7 @@ static const struct rule_opts rule_nfc_tcp_ip4[] = { { "dst-port", OPT_BE16, NFC_FLAG_DPORT, offsetof(struct ethtool_rx_flow_spec, h_u.tcp_ip4_spec.pdst), offsetof(struct ethtool_rx_flow_spec, m_u.tcp_ip4_spec.pdst) }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -685,7 +686,7 @@ static const struct rule_opts rule_nfc_esp_ip4[] = { { "spi", OPT_BE32, NFC_FLAG_SPI, offsetof(struct ethtool_rx_flow_spec, h_u.esp_ip4_spec.spi), offsetof(struct ethtool_rx_flow_spec, m_u.esp_ip4_spec.spi) }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -728,7 +729,7 @@ static const struct rule_opts rule_nfc_usr_ip4[] = { { "dst-port", OPT_BE16, NFC_FLAG_DPORT, offsetof(struct ethtool_rx_flow_spec, h_u.usr_ip4_spec.l4_4_bytes) + 2, offsetof(struct ethtool_rx_flow_spec, m_u.usr_ip4_spec.l4_4_bytes) + 2 }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -762,7 +763,7 @@ static const struct rule_opts rule_nfc_tcp_ip6[] = { { "dst-port", OPT_BE16, NFC_FLAG_DPORT, offsetof(struct ethtool_rx_flow_spec, h_u.tcp_ip6_spec.pdst), offsetof(struct ethtool_rx_flow_spec, m_u.tcp_ip6_spec.pdst) }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -793,7 +794,7 @@ static const struct rule_opts rule_nfc_esp_ip6[] = { { "spi", OPT_BE32, NFC_FLAG_SPI, offsetof(struct ethtool_rx_flow_spec, h_u.esp_ip6_spec.spi), offsetof(struct ethtool_rx_flow_spec, m_u.esp_ip6_spec.spi) }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -836,7 +837,7 @@ static const struct rule_opts rule_nfc_usr_ip6[] = { { "dst-port", OPT_BE16, NFC_FLAG_DPORT, offsetof(struct ethtool_rx_flow_spec, h_u.usr_ip6_spec.l4_4_bytes) + 2, offsetof(struct ethtool_rx_flow_spec, m_u.usr_ip6_spec.l4_4_bytes) + 2 }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -864,7 +865,7 @@ static const struct rule_opts rule_nfc_ether[] = { { "proto", OPT_BE16, NFC_FLAG_PROTO, offsetof(struct ethtool_rx_flow_spec, h_u.ether_spec.h_proto), offsetof(struct ethtool_rx_flow_spec, m_u.ether_spec.h_proto) }, - { "action", OPT_U64, NFC_FLAG_RING, + { "action", OPT_ACTION, NFC_FLAG_RING, offsetof(struct ethtool_rx_flow_spec, ring_cookie), -1 }, { "loc", OPT_U32, NFC_FLAG_LOC, offsetof(struct ethtool_rx_flow_spec, location), -1 }, @@ -948,6 +949,40 @@ static int rxclass_get_ether(char *str, unsigned char *val) return 0; } +static int rxclass_get_action(char *str, unsigned long long *val) +{ + const long long max_queue = ~0ULL >> 32; + const long long max_vf = ~0ULL >> 8; + char *end_vf, *end_queue; + unsigned long long queue, vf; + + /* If there is no '/' just read the full value like a u64 */ + if (!strchr(str, '/')) + return rxclass_get_ulong(str, val, 64); + + errno = 0; + + vf = strtoull(str, &end_vf, 0); + + /* + * Ensure that we consume everything up to the slash, and that there + * is more than the empty string following the slash. + */ + if ((*end_vf != '/') || !(*(end_vf + 1)) || errno || (vf > max_vf)) + return -1; + + errno = 0; + + queue = strtoull(end_vf + 1, &end_queue, 0); + + if (*end_queue || errno || (queue > max_queue)) + return -1; + + *val = (vf << ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF) | queue; + + return 0; +} + static int rxclass_get_val(char *str, unsigned char *p, u32 *flags, const struct rule_opts *opt) { @@ -1070,6 +1105,14 @@ static int rxclass_get_val(char *str, unsigned char *p, u32 *flags, memcpy(&p[opt->moffset], &mask, ETH_ALEN); break; } + case OPT_ACTION: { + unsigned long long val; + err = rxclass_get_action(str, &val); + if (err) + return -1; + *(u64 *)&p[opt->offset] = (u64)val; + break; + } case OPT_NONE: default: return -1; @@ -1183,6 +1226,7 @@ static int rxclass_get_mask(char *str, unsigned char *p, memcpy(&p[opt->moffset], val, ETH_ALEN); break; } + case OPT_ACTION: case OPT_NONE: default: return -1; -- 2.11.0.rc2.152.g4d04e67 ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-06 12:40 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-06 12:40 [PATCH RFC v1 0/2] ethtool: add support for VF in ring_cookie Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 1/2] rxclass: use ethtool_get_flow_spec_ring to display queue and VF Jacob Keller 2016-12-06 12:40 ` [PATCH RFC v1 2/2] ethtool: add support to specify ring_cookie as VF/queue Jacob Keller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).