| Message ID | 20191208132304.15521-16-elibr@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 08.12.2019 14:23, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > --- > Documentation/howto/dpdk.rst | 1 + > NEWS | 2 +- > lib/netdev-dpdk.c | 2 ++ > lib/netdev-offload-dpdk.c | 4 +--- > 4 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index f62ce82af..6cedd7f63 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are: > Supported actions for hardware offload are: > > - Output (a single output, as the last action). > +- Drop. > > Further Reading > --------------- > diff --git a/NEWS b/NEWS > index c430999a0..d019e066f 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,7 +26,7 @@ Post-v2.12.0 > * DPDK ring ports (dpdkr) are deprecated and will be removed in next > releases. > * Add support for DPDK 19.11. > - * Add hardware offload support for output actions (experimental). > + * Add hardware offload support for output and drop actions (experimental). > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index d9a2c2004..872a45e75 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) > } else { > ds_put_cstr(s, " Port-id = null\n"); > } > + } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { > + ds_put_cstr(s, "rte flow drop action\n"); > } else { > ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); > } > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index 0b9087192..bffb69cad 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev, > } > > if (nl_actions_len == 0) { > - VLOG_DBG_RL(&error_rl, > - "Unsupported action type drop"); > - return -1; > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL); Do we need an explicit drop action? What will happen if HW will have only packet modification actions or will not have actions at all? Will it send packet to driver for processing or it will drop it? Do we need to finish all the flows that doesn't end with DROP/PORT_ID actions with explicit drop action? Asking because it's possible to have such flows in OVS.
On 12/10/2019 9:10 PM, Ilya Maximets wrote: > On 08.12.2019 14:23, Eli Britstein wrote: >> Signed-off-by: Eli Britstein <elibr@mellanox.com> >> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> >> --- >> Documentation/howto/dpdk.rst | 1 + >> NEWS | 2 +- >> lib/netdev-dpdk.c | 2 ++ >> lib/netdev-offload-dpdk.c | 4 +--- >> 4 files changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >> index f62ce82af..6cedd7f63 100644 >> --- a/Documentation/howto/dpdk.rst >> +++ b/Documentation/howto/dpdk.rst >> @@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are: >> Supported actions for hardware offload are: >> >> - Output (a single output, as the last action). >> +- Drop. >> >> Further Reading >> --------------- >> diff --git a/NEWS b/NEWS >> index c430999a0..d019e066f 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -26,7 +26,7 @@ Post-v2.12.0 >> * DPDK ring ports (dpdkr) are deprecated and will be removed in next >> releases. >> * Add support for DPDK 19.11. >> - * Add hardware offload support for output actions (experimental). >> + * Add hardware offload support for output and drop actions (experimental). >> >> v2.12.0 - 03 Sep 2019 >> --------------------- >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index d9a2c2004..872a45e75 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) >> } else { >> ds_put_cstr(s, " Port-id = null\n"); >> } >> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { >> + ds_put_cstr(s, "rte flow drop action\n"); >> } else { >> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); >> } >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> index 0b9087192..bffb69cad 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev, >> } >> >> if (nl_actions_len == 0) { >> - VLOG_DBG_RL(&error_rl, >> - "Unsupported action type drop"); >> - return -1; >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL); > Do we need an explicit drop action? > What will happen if HW will have only packet modification actions > or will not have actions at all? Will it send packet to driver for > processing or it will drop it? > Do we need to finish all the flows that doesn't end with DROP/PORT_ID > actions with explicit drop action? > Asking because it's possible to have such flows in OVS. The HW flow must have a "fate" action. A fate action can be RSS as done before, or DROP/PORT_ID, that are used in this series, but can also be JUMP to another table (flow_attr.group). In OVS, as far as I saw, there is a similar logic. For example, if I configure OF flow that just rewrite some fields, and no "fate" action, in DP, it ends with "drop", which aligns with the HW. What other kinds of flows in OVS do you mean?
On 11.12.2019 17:01, Eli Britstein wrote: > > On 12/10/2019 9:10 PM, Ilya Maximets wrote: >> On 08.12.2019 14:23, Eli Britstein wrote: >>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> >>> --- >>> Documentation/howto/dpdk.rst | 1 + >>> NEWS | 2 +- >>> lib/netdev-dpdk.c | 2 ++ >>> lib/netdev-offload-dpdk.c | 4 +--- >>> 4 files changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >>> index f62ce82af..6cedd7f63 100644 >>> --- a/Documentation/howto/dpdk.rst >>> +++ b/Documentation/howto/dpdk.rst >>> @@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are: >>> Supported actions for hardware offload are: >>> >>> - Output (a single output, as the last action). >>> +- Drop. >>> >>> Further Reading >>> --------------- >>> diff --git a/NEWS b/NEWS >>> index c430999a0..d019e066f 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -26,7 +26,7 @@ Post-v2.12.0 >>> * DPDK ring ports (dpdkr) are deprecated and will be removed in next >>> releases. >>> * Add support for DPDK 19.11. >>> - * Add hardware offload support for output actions (experimental). >>> + * Add hardware offload support for output and drop actions (experimental). >>> >>> v2.12.0 - 03 Sep 2019 >>> --------------------- >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>> index d9a2c2004..872a45e75 100644 >>> --- a/lib/netdev-dpdk.c >>> +++ b/lib/netdev-dpdk.c >>> @@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) >>> } else { >>> ds_put_cstr(s, " Port-id = null\n"); >>> } >>> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { >>> + ds_put_cstr(s, "rte flow drop action\n"); >>> } else { >>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); >>> } >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>> index 0b9087192..bffb69cad 100644 >>> --- a/lib/netdev-offload-dpdk.c >>> +++ b/lib/netdev-offload-dpdk.c >>> @@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev, >>> } >>> >>> if (nl_actions_len == 0) { >>> - VLOG_DBG_RL(&error_rl, >>> - "Unsupported action type drop"); >>> - return -1; >>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL); >> Do we need an explicit drop action? >> What will happen if HW will have only packet modification actions >> or will not have actions at all? Will it send packet to driver for >> processing or it will drop it? >> Do we need to finish all the flows that doesn't end with DROP/PORT_ID >> actions with explicit drop action? >> Asking because it's possible to have such flows in OVS. > > The HW flow must have a "fate" action. A fate action can be RSS as done > before, or DROP/PORT_ID, that are used in this series, but can also be > JUMP to another table (flow_attr.group). > > In OVS, as far as I saw, there is a similar logic. For example, if I > configure OF flow that just rewrite some fields, and no "fate" action, > in DP, it ends with "drop", which aligns with the HW. > > What other kinds of flows in OVS do you mean? OVS is able to create datapath flow with pure tunnel push wihout further processing, i.e. tunnel push is the last action. I've seen this and this was caused by some highly complex configuration with SDN controller, so, I'm not sure how to reproduce that. But that is the fact. OVS is able to generate such flows. Userspace datapath had issues with that case and we had a mbuf leak in this case that drained the mempool quickly. How HW will react on this? Will it be just a flow installation failure or we'll get those packets in SW for further processing? BTW, I'm not sure if other types of similar flows are possible, I noticed tunnel push only because we had a problem with it in userspace datapath.
On 12/11/2019 8:01 PM, Ilya Maximets wrote: > On 11.12.2019 17:01, Eli Britstein wrote: >> On 12/10/2019 9:10 PM, Ilya Maximets wrote: >>> On 08.12.2019 14:23, Eli Britstein wrote: >>>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>>> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> >>>> --- >>>> Documentation/howto/dpdk.rst | 1 + >>>> NEWS | 2 +- >>>> lib/netdev-dpdk.c | 2 ++ >>>> lib/netdev-offload-dpdk.c | 4 +--- >>>> 4 files changed, 5 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >>>> index f62ce82af..6cedd7f63 100644 >>>> --- a/Documentation/howto/dpdk.rst >>>> +++ b/Documentation/howto/dpdk.rst >>>> @@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are: >>>> Supported actions for hardware offload are: >>>> >>>> - Output (a single output, as the last action). >>>> +- Drop. >>>> >>>> Further Reading >>>> --------------- >>>> diff --git a/NEWS b/NEWS >>>> index c430999a0..d019e066f 100644 >>>> --- a/NEWS >>>> +++ b/NEWS >>>> @@ -26,7 +26,7 @@ Post-v2.12.0 >>>> * DPDK ring ports (dpdkr) are deprecated and will be removed in next >>>> releases. >>>> * Add support for DPDK 19.11. >>>> - * Add hardware offload support for output actions (experimental). >>>> + * Add hardware offload support for output and drop actions (experimental). >>>> >>>> v2.12.0 - 03 Sep 2019 >>>> --------------------- >>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>>> index d9a2c2004..872a45e75 100644 >>>> --- a/lib/netdev-dpdk.c >>>> +++ b/lib/netdev-dpdk.c >>>> @@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) >>>> } else { >>>> ds_put_cstr(s, " Port-id = null\n"); >>>> } >>>> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { >>>> + ds_put_cstr(s, "rte flow drop action\n"); >>>> } else { >>>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); >>>> } >>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>>> index 0b9087192..bffb69cad 100644 >>>> --- a/lib/netdev-offload-dpdk.c >>>> +++ b/lib/netdev-offload-dpdk.c >>>> @@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev, >>>> } >>>> >>>> if (nl_actions_len == 0) { >>>> - VLOG_DBG_RL(&error_rl, >>>> - "Unsupported action type drop"); >>>> - return -1; >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL); >>> Do we need an explicit drop action? >>> What will happen if HW will have only packet modification actions >>> or will not have actions at all? Will it send packet to driver for >>> processing or it will drop it? >>> Do we need to finish all the flows that doesn't end with DROP/PORT_ID >>> actions with explicit drop action? >>> Asking because it's possible to have such flows in OVS. >> The HW flow must have a "fate" action. A fate action can be RSS as done >> before, or DROP/PORT_ID, that are used in this series, but can also be >> JUMP to another table (flow_attr.group). >> >> In OVS, as far as I saw, there is a similar logic. For example, if I >> configure OF flow that just rewrite some fields, and no "fate" action, >> in DP, it ends with "drop", which aligns with the HW. >> >> What other kinds of flows in OVS do you mean? > OVS is able to create datapath flow with pure tunnel push wihout further > processing, i.e. tunnel push is the last action. I've seen this and > this was caused by some highly complex configuration with SDN controller, > so, I'm not sure how to reproduce that. But that is the fact. OVS is > able to generate such flows. Userspace datapath had issues with that case > and we had a mbuf leak in this case that drained the mempool quickly. > How HW will react on this? Will it be just a flow installation failure > or we'll get those packets in SW for further processing? I suppose it will be a failure in PMD level and we'll try to do MARK/RSS. The HW processes all the packets by trying to match on its rules. If there is a hit, it processes by the actions. If a suitable rule is not found (miss), the packet arrives to SW. So, besides the error in log, I don't see a fatal here. > > BTW, I'm not sure if other types of similar flows are possible, I noticed > tunnel push only because we had a problem with it in userspace datapath. OK. If you know any kind of reproduction scenario, please let me know. Thanks.
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst index f62ce82af..6cedd7f63 100644 --- a/Documentation/howto/dpdk.rst +++ b/Documentation/howto/dpdk.rst @@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are: Supported actions for hardware offload are: - Output (a single output, as the last action). +- Drop. Further Reading --------------- diff --git a/NEWS b/NEWS index c430999a0..d019e066f 100644 --- a/NEWS +++ b/NEWS @@ -26,7 +26,7 @@ Post-v2.12.0 * DPDK ring ports (dpdkr) are deprecated and will be removed in next releases. * Add support for DPDK 19.11. - * Add hardware offload support for output actions (experimental). + * Add hardware offload support for output and drop actions (experimental). v2.12.0 - 03 Sep 2019 --------------------- diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index d9a2c2004..872a45e75 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) } else { ds_put_cstr(s, " Port-id = null\n"); } + } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { + ds_put_cstr(s, "rte flow drop action\n"); } else { ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); } diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 0b9087192..bffb69cad 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev, } if (nl_actions_len == 0) { - VLOG_DBG_RL(&error_rl, - "Unsupported action type drop"); - return -1; + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL); } add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);