| Message ID | 20191218144110.18653-1-elibr@mellanox.com |
|---|---|
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 18.12.2019 15:40, Eli Britstein wrote: > Currently, netdev datapath offload only accelerates the flow match > sequence by associating a mark per flow. This series introduces the full > offload of netdev datapath flows by having the HW also perform the flow > actions. > > This series adds HW offload for output, drop, set MAC, set IPv4, set > TCP/UDP ports and tunnel push actions using the DPDK rte_flow API. > > v1 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/614511472 > v2 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/619262148 > v3 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/622253870 > v4 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/625654160 > v5 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/626692202 > > v1-v2: > - fallback to mark/rss in case of any failure of action offload. > - dp_layer changed to "in_hw" in case the flow is offloaded. > - using netdev_ports_get instead of dp_netdev_lookup_port in stats reading. > - using flow_dump_next API instead of a new API. > - validity checks for last action of output and clone. > - count action should be done for drop as well. > - validity check for output action. > - added doc in Documentation/howto/dpdk.rst. > v2-v3: > - removed refactoring for netdev-offload-dpdk-flow.c file. > - dropped flush commits. > - dbg prints rate-limited, and outside lock. > - added validity check for output action - same flow_api handle for both netdevs. > - added a mutex to protect possible concurrent deletion/querying of a flow. > - statistics are collected using flow_get API. > - added more documentation in Documentation/howto/dpdk.rst. > v3-v4: > - debug prints moved to netdev-offload-dpdk.c. > - flow get returns full stats, not diff. > - removed additional fields from offload_info and dp_netdev_flow. > - read HW stats during dpif_netdev_flow_get as well as > dpif_netdev_flow_dump_next. > - moved actions offload framework just before support commit. > - fixed memory leak in rewrite code. > - dropped clone commit - will be posted in next series. > v4-v5: > - statistics collecting changed to populate dp_layer and offloaded > attributes. dp_layer changed to "dpdk" for fully offloaded by dpdk. > - display offloaded:partial for partially offloaded flows. > - support filter types "dpdk" and "partially-offloaded" in > dpctl/dump-flows type=X. > - simplify code readability of rewrite commits. > > Eli Britstein (17): > netdev-offload-dpdk: Refactor flow patterns > netdev-offload-dpdk: Refactor action items freeing scheme > netdev-offload-dpdk: Dynamically allocate pattern items > netdev-offload-dpdk: Improve HW offload flow debuggability > netdev-dpdk: Introduce rte flow query function > netdev-offload-dpdk: Return UFID-rte_flow entry in find method > netdev-offload-dpdk: Framework for actions offload > netdev-offload-dpdk: Implement flow get method > dpctl: Support dump-flows filters "dpdk" and "partially-offloaded" > dpif-netdev: Populate dpif class field in offload struct > netdev-dpdk: Getter function for dpdk port id API > netdev-offload: Introduce a function to validate same flow api handle > netdev-offload-dpdk: Support offload of output action > netdev-offload-dpdk: Support offload of drop action > netdev-offload-dpdk: Support offload of set MAC actions > netdev-offload-dpdk: Support offload of set IPv4 actions > netdev-offload-dpdk: Support offload of set TCP/UDP ports actions > > Ophir Munk (1): > dpif-netdev: Update offloaded flows statistics > > Documentation/howto/dpdk.rst | 21 +- > NEWS | 5 + > lib/dpctl.c | 97 +++-- > lib/dpctl.man | 2 + > lib/dpif-netdev.c | 81 +++- > lib/netdev-dpdk.c | 48 +++ > lib/netdev-dpdk.h | 8 + > lib/netdev-offload-dpdk.c | 957 ++++++++++++++++++++++++++++++++---------- > lib/netdev-offload-provider.h | 1 + > lib/netdev-offload.c | 12 + > 10 files changed, 965 insertions(+), 267 deletions(-) > I really roughly looked through the series and made a couple of comments. Also, there are few checkpatch warnings about line lengths. And it's still unclear what is the profit from dynamic allocation of flow patters (I didn't decide if I like or dislike this change). One more thing I want to mention is that we're likely going to have explicit drop actions with this patch: https://patchwork.ozlabs.org/patch/1211955/ It's unclear right now which patches will be applied first, but anyway netdev-offload-dpdk will need to support this OVS_ACTION_ATTR_DROP somehow. Should be easy to support if we'll ignore the drop reason. Statistics update might be harder and, I think, not necessary right now. Just noticing for you to keep in mind/prepare for the change. BTW, I'm traveling starting from tomorrow (today already) until the end of next week, so I'll most likely not be able to review for that period. However, should still be reachable, but slower in replies. Best regards, Ilya Maximets.
On 12/19/2019 1:11 AM, Ilya Maximets wrote: > I really roughly looked through the series and made a couple of comments. > Also, there are few checkpatch warnings about line lengths. And it's > still unclear what is the profit from dynamic allocation of flow patters > (I didn't decide if I like or dislike this change). Actions are dynamically allocated, allowing just passing "struct flow_actions *actions" to various functions, without propagating a struct with all the actions, and also allows incrementally adding more actions without changing such struct. With patterns, I'd like to keep it, so it is more aligned with actions dynamic allocation, with the same benefits, though such propagating can be currently only to one function. The next series on top of this one I will submit is VXLAN offloading, which will introduce more functions of patterns, enjoying the same benefits. > > One more thing I want to mention is that we're likely going to have > explicit drop actions with this patch: > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F1211955%2F&data=02%7C01%7Celibr%40mellanox.com%7C34c5e23e65124344bceb08d7840f9474%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637123074772939197&sdata=Mmm%2BJY9TKjVabSWjowvXqeei76JmtP%2BaH3x2%2B2eCaVE%3D&reserved=0 > It's unclear right now which patches will be applied first, but anyway > netdev-offload-dpdk will need to support this OVS_ACTION_ATTR_DROP > somehow. Should be easy to support if we'll ignore the drop reason. > Statistics update might be harder and, I think, not necessary right now. > Just noticing for you to keep in mind/prepare for the change. Thanks. OK. I noticed that this patch doesn't change netdev-offload-tc.c. Don't you think this patch should be changed to use this new attribute there too? > > BTW, I'm traveling starting from tomorrow (today already) until the end > of next week, so I'll most likely not be able to review for that period. > However, should still be reachable, but slower in replies. bon voyage > > Best regards, Ilya Maximets.