On 11/12/2024 06:53, Allen Chen wrote: > Hi Kevin, > > I agree with the comments, and please make the changes while applying the > code. > Thanks very much. >
Thanks Allen. Applied and pushed to main. Kevin. > Best wishes. > -----邮件原件----- > 发件人: Kevin Traynor <[email protected]> > 发送时间: 2024年12月11日 3:55 > 收件人: Allen Chen <[email protected]>; [email protected] > 主题: Re: [ovs-dev] [PATCH v3] netdev_offload_dpdk: Support icmpv6 offload. > > External Mail: This email originated from OUTSIDE of the organization! > Do not click links, open attachments or provide ANY information unless you > recognize the sender and know the content is safe. > > > On 12/11/2024 11:45, Allen Chen via dev wrote: >> Support icmpv6 offload >>> Signed-off-by: Allen Chen <[email protected]> >> > > Hi Allen, > > In general it looks good. I was able to test the code and it looks to be > creating the correct match rule. > > Just a few minor comments on the code. If you agree with the comments, I can > make the changes while applying the code. Or if you prefer to discuss more or > send a new patch that is fine too. > > thanks, > Kevin. > > -- > > The title should have hyphens, not underscores and though the description is > accurate, it would be good to provide a little more context. Suggest > something like: > > netdev-offload-dpdk: Support ICMPv6 offload. > > Add support for offloading packet match on ICMPv6 header using rte_flow API. > >> --- >> //add ovs flow table >> [root@localhost ~]# ovs-ofctl add-flow br-jmnd >> in_port=dpdk0,dl_type=0x86dd,nw_proto=58,action=output:net0 >> >> //dump ovs flow table >> [root@localhost ~]# ovs-ofctl dump-flows br-jmnd cookie=0x0, >> duration=759.207s, table=0, n_packets=303, n_bytes=37572, >> icmp6,in_port=dpdk0 actions=output:net0 cookie=0x0, >> duration=732.399s, table=0, n_packets=0, n_bytes=0, icmp6,in_port=net0 >> actions=output:dpdk0 >> >> //set 'ovs-appctl vlog/set netdev_offload_dpdk:dbg' and cat ovs-vswitchd.log >> 2018-06-22T20:02:10.744Z|00003|netdev_offload_dpdk(hw_offload5)|DBG|dpdk0: >> rte_flow 0x10037b0c0 flow create 0 priority 0 group 0 transfer pattern eth >> type is 0x86dd has_vlan is 0 / ipv6 proto is 58 has_frag_ext is 0 / icmp6 / >> end actions count / port_id original 0 id 2 / end >> 2018-06-22T20:02:10.744Z|00004|netdev_offload_dpdk(hw_offload5)|DBG|dp >> dk0/dpdk0: installed flow 0x10037b0c0 by ufid >> 0ef1818a-eca1-40fc-a105-97046fcbbc3f >> 2018-06-22T20:02:50.968Z|00005|netdev_offload_dpdk(hw_offload5)|DBG|dpdk0/dpdk0: >> rte_flow 0x10037b0c0 flow destroy 0 ufid >> 0ef1818a-eca1-40fc-a105-97046fcbbc3f >> 2018-06-22T20:09:01.696Z|00006|netdev_offload_dpdk(hw_offload5)|DBG|dpdk0: >> rte_flow 0x10037b200 flow create 0 priority 0 group 0 transfer pattern eth >> type is 0x86dd has_vlan is 0 / ipv6 proto is 58 has_frag_ext is 0 / icmp6 / >> end actions count / port_id original 0 id 2 / end >> 2018-06-22T20:09:01.696Z|00007|netdev_offload_dpdk(hw_offload5)|DBG|dp >> dk0/dpdk0: installed flow 0x10037b200 by ufid >> 0ef1818a-eca1-40fc-a105-97046fcbbc3f >> 2018-06-22T20:11:21.904Z|00008|netdev_offload_dpdk(hw_offload5)|DBG|dp >> dk0/dpdk0: rte_flow 0x10037b200 flow destroy 0 ufid >> 0ef1818a-eca1-40fc-a105-97046fcbbc3f >> >> //confirm the flow is offloaded >> [root@localhost ~]# ovs-appctl dpctl/dump-flows -m | grep offloaded >> ufid:51a14f16-7adc-4c5d-b97d-0c5487ef9777, >> mega_ufid:0ef1818a-eca1-40fc-a105-97046fcbbc3f, >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0), >> >> dp_hash(0/0),in_port(dpdk0),packet_type(ns=0,id=0),eth(src=00:00:00:12:30:10/00:00:00:00:00:00,dst=00:00:00:13:40:20/00:00:00:00:00:00),eth_type(0x86dd),ipv6(src=2001::2/::,dst=2001::1:f1:11/::,label=0/0,proto=58,tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=129/0,code=0/0), >> packets:57, bytes:7068, dpe_packets:57, dpe_bytes:7068,used:0.000s, >> offloaded:yes, dp:dpdk, actions:net0,dp-extra-info:miniflow_bits(4,1) >> >> //gdb offload process >> (gdb) >> at ../drivers/net/jmnd/jmnd_flow/jmnd_flow.c:622 >> at ../drivers/net/jmnd/jmnd_flow/jmnd_flow.c:1765 >> at lib/netdev-offload-dpdk.c:1277 >> act_vars=0xfffe7fffb818) at lib/netdev-offload-dpdk.c:2823 >> at lib/netdev-offload-dpdk.c:2868 >> at lib/netdev-offload-dpdk.c:3002 >> at lib/netdev-offload.c:264 >> (gdb) p patterns[0] >> $1 = {type = RTE_FLOW_ITEM_TYPE_ETH, spec = 0xfffe74003300, last = >> 0x0, mask = 0xfffe74003250} >> (gdb) p patterns[1] >> $2 = {type = RTE_FLOW_ITEM_TYPE_IPV6, spec = 0xfffe740035b0, last = >> 0x0, mask = 0xfffe740035f0} >> (gdb) p patterns[2] >> $3 = {type = RTE_FLOW_ITEM_TYPE_ICMP6, spec = 0xfffe74002a50, last = >> 0x0, mask = 0xfffe74002ce0} >> (gdb) p patterns[3] >> $4 = {type = RTE_FLOW_ITEM_TYPE_END, spec = 0x0, last = 0x0, mask = >> 0x0} >> (gdb) p action[0] >> Structure has no component named operator[]. >> (gdb) p actions[0] >> $5 = {type = RTE_FLOW_ACTION_TYPE_COUNT, conf = 0xfffe740037c0} >> (gdb) p actions[1] >> $6 = {type = RTE_FLOW_ACTION_TYPE_PORT_ID, conf = 0xfffe74003870} >> (gdb) p actions[2] >> $7 = {type = RTE_FLOW_ACTION_TYPE_END, conf = 0x0} >> (gdb) p/x *(struct rte_flow_item_icmp6 *)0xfffe74002a50 >> $8 = {type = 0x81, code = 0x0, checksum = 0x0} >> (gdb) p/x *(struct rte_flow_item_icmp6 *)0xfffe74002ce0 >> $9 = {type = 0x0, code = 0x0, checksum = 0x0} >> --- >> NEWS | 5 +++++ >> lib/netdev-offload-dpdk.c | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/NEWS b/NEWS >> index c2443a999..2a9099851 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -1,3 +1,8 @@ >> +v3.4.0 - 12 Nov 2024 >> +--------------------- >> + - Add support for ICMPV6 offload. >> + > > We need to be more specific as there are different types of offloads. > So it can go in the 'Post-v3.4.0' section with the other 'DPDK:' entry i.e. > > - DPDK: > * OVS validated with DPDK 23.11.2. > * Add hardware offload support for matching ICMPv6 protocol > (experimental). > >> + >> Post-v3.4.0 >> -------------------- >> - Userspace datapath: >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> index 1a6e100ff..1ec5d175b 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -495,6 +495,26 @@ dump_flow_pattern(struct ds *s, >> icmp_mask->hdr.icmp_code, 0); >> } >> ds_put_cstr(s, "/ "); >> + } else if (item->type == RTE_FLOW_ITEM_TYPE_ICMP6) { >> + const struct rte_flow_item_icmp6 *icmp6_spec = item->spec; >> + const struct rte_flow_item_icmp6 *icmp6_mask = item->mask; >> + >> + ds_put_cstr(s, "icmpv6 "); >> + if (icmp6_spec) { >> + if (!icmp6_mask) { >> + icmp6_mask = &rte_flow_item_icmp6_mask; >> + } >> + DUMP_PATTERN_ITEM(icmp6_mask->type, false, "type", >> + "%"PRIu8, icmp6_spec->type, >> + icmp6_mask->type, 0); >> + DUMP_PATTERN_ITEM(icmp6_mask->code, false, "code", >> + "%"PRIu8, icmp6_spec->code, >> + icmp6_mask->code, 0); > >> + DUMP_PATTERN_ITEM(icmp6_mask->checksum, false, "checksum", >> + "%"PRIu8, icmp6_spec->checksum, >> + icmp6_mask->checksum, 0); >> + } > > The checksum does not need to be dumped. > >> + ds_put_cstr(s, "/ "); >> } else if (item->type == RTE_FLOW_ITEM_TYPE_TCP) { >> const struct rte_flow_item_tcp *tcp_spec = item->spec; >> const struct rte_flow_item_tcp *tcp_mask = item->mask; @@ >> -1608,6 +1628,7 @@ parse_flow_match(struct netdev *netdev, >> >> if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP && >> proto != IPPROTO_SCTP && proto != IPPROTO_TCP && >> + proto != IPPROTO_ICMPV6 && >> (match->wc.masks.tp_src || >> match->wc.masks.tp_dst || >> match->wc.masks.tcp_flags)) { @@ -1684,6 +1705,22 @@ >> parse_flow_match(struct netdev *netdev, >> consumed_masks->tp_dst = 0; >> >> add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ICMP, spec, >> mask, NULL); >> + } else if (proto == IPPROTO_ICMPV6) { >> + struct rte_flow_item_icmp6 *spec, *mask; >> + >> + spec = xzalloc(sizeof *spec); >> + mask = xzalloc(sizeof *mask); >> + >> + spec->type = (uint8_t) ntohs(match->flow.tp_src); >> + spec->code = (uint8_t) ntohs(match->flow.tp_dst); >> + >> + mask->type = (uint8_t) ntohs(match->wc.masks.tp_src); >> + mask->code = (uint8_t) ntohs(match->wc.masks.tp_dst); >> + >> + consumed_masks->tp_src = 0; >> + consumed_masks->tp_dst = 0; >> + >> + add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ICMP6, spec, >> + mask, NULL); >> } >> >> add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL, >> NULL); > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
