Regards,
Asaf Penso
> -----Original Message-----
> From: Ilya Maximets <[email protected]>
> Sent: Monday, February 4, 2019 4:14 PM
> To: Asaf Penso <[email protected]>; [email protected]
> Cc: Roni Bar Yanai <[email protected]>; Stokes, Ian
> <[email protected]>
> Subject: Re: [ovs-dev] netdev-dpdk: Memset rte_flow_item on a need basis.
>
> On 04.02.2019 15:50, Asaf Penso wrote:
> > In netdev_dpdk_add_rte_flow_offload function different rte_flow_item
> > are created as part of the pattern matching.
> >
> > For most of them, there is a check whether the wildcards are not zero.
> > In case of zero, nothing is being done with the rte_flow_item.
> >
> > Befor the wildcard check, and regardless of the result, the
> > rte_flow_item is being memset.
> >
> > The patch moves the memset function only if the condition of the
> > wildcard is true, thus saving cycles of memset if not needed.
>
> Structures are actually used only inside their 'if' blocks.
> IMHO, it's better to move the definitions inside too.
>
Inside each 'if' block there is a call to add_flow_pattern that updates the
pattern's pointers to these structs.
Having them defined inside the block will cause their value to be corrupted
once we exit the block.
They are needed all the way until rte_flow_create call.
> >
> > Signed-off-by: Asaf Penso <[email protected]>
> > ---
> > lib/netdev-dpdk.c | 28 ++++++++++++++--------------
> > 1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 4bf0ca9..5216b74 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -4570,10 +4570,10 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev *netdev,
> > /* Eth */
> > struct rte_flow_item_eth eth_spec;
> > struct rte_flow_item_eth eth_mask;
> > - memset(ð_spec, 0, sizeof(eth_spec));
> > - memset(ð_mask, 0, sizeof(eth_mask));
> > if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
> > !eth_addr_is_zero(match->wc.masks.dl_dst)) {
> > + memset(ð_spec, 0, sizeof(eth_spec));
> > + memset(ð_mask, 0, sizeof(eth_mask));
> > rte_memcpy(ð_spec.dst, &match->flow.dl_dst,
> sizeof(eth_spec.dst));
> > rte_memcpy(ð_spec.src, &match->flow.dl_src,
> sizeof(eth_spec.src));
> > eth_spec.type = match->flow.dl_type; @@ -4600,9 +4600,9 @@
> > netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,
> > /* VLAN */
> > struct rte_flow_item_vlan vlan_spec;
> > struct rte_flow_item_vlan vlan_mask;
> > - memset(&vlan_spec, 0, sizeof(vlan_spec));
> > - memset(&vlan_mask, 0, sizeof(vlan_mask));
> > if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
> > + memset(&vlan_spec, 0, sizeof(vlan_spec));
> > + memset(&vlan_mask, 0, sizeof(vlan_mask));
> > vlan_spec.tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
> > vlan_mask.tci = match->wc.masks.vlans[0].tci &
> > ~htons(VLAN_CFI);
> >
> > @@ -4617,9 +4617,9 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev *netdev,
> > uint8_t proto = 0;
> > struct rte_flow_item_ipv4 ipv4_spec;
> > struct rte_flow_item_ipv4 ipv4_mask;
> > - memset(&ipv4_spec, 0, sizeof(ipv4_spec));
> > - memset(&ipv4_mask, 0, sizeof(ipv4_mask));
> > if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
> > + memset(&ipv4_spec, 0, sizeof(ipv4_spec));
> > + memset(&ipv4_mask, 0, sizeof(ipv4_mask));
> >
> > ipv4_spec.hdr.type_of_service = match->flow.nw_tos;
> > ipv4_spec.hdr.time_to_live = match->flow.nw_ttl;
> > @@ -4662,9 +4662,9 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev
> > *netdev,
> >
> > struct rte_flow_item_tcp tcp_spec;
> > struct rte_flow_item_tcp tcp_mask;
> > - memset(&tcp_spec, 0, sizeof(tcp_spec));
> > - memset(&tcp_mask, 0, sizeof(tcp_mask));
> > if (proto == IPPROTO_TCP) {
> > + memset(&tcp_spec, 0, sizeof(tcp_spec));
> > + memset(&tcp_mask, 0, sizeof(tcp_mask));
> > tcp_spec.hdr.src_port = match->flow.tp_src;
> > tcp_spec.hdr.dst_port = match->flow.tp_dst;
> > tcp_spec.hdr.data_off = ntohs(match->flow.tcp_flags) >> 8;
> > @@ -4687,9 +4687,9 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev
> > *netdev,
> >
> > struct rte_flow_item_udp udp_spec;
> > struct rte_flow_item_udp udp_mask;
> > - memset(&udp_spec, 0, sizeof(udp_spec));
> > - memset(&udp_mask, 0, sizeof(udp_mask));
> > if (proto == IPPROTO_UDP) {
> > + memset(&udp_spec, 0, sizeof(udp_spec));
> > + memset(&udp_mask, 0, sizeof(udp_mask));
> > udp_spec.hdr.src_port = match->flow.tp_src;
> > udp_spec.hdr.dst_port = match->flow.tp_dst;
> >
> > @@ -4708,9 +4708,9 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev
> > *netdev,
> >
> > struct rte_flow_item_sctp sctp_spec;
> > struct rte_flow_item_sctp sctp_mask;
> > - memset(&sctp_spec, 0, sizeof(sctp_spec));
> > - memset(&sctp_mask, 0, sizeof(sctp_mask));
> > if (proto == IPPROTO_SCTP) {
> > + memset(&sctp_spec, 0, sizeof(sctp_spec));
> > + memset(&sctp_mask, 0, sizeof(sctp_mask));
> > sctp_spec.hdr.src_port = match->flow.tp_src;
> > sctp_spec.hdr.dst_port = match->flow.tp_dst;
> >
> > @@ -4729,9 +4729,9 @@ netdev_dpdk_add_rte_flow_offload(struct
> netdev
> > *netdev,
> >
> > struct rte_flow_item_icmp icmp_spec;
> > struct rte_flow_item_icmp icmp_mask;
> > - memset(&icmp_spec, 0, sizeof(icmp_spec));
> > - memset(&icmp_mask, 0, sizeof(icmp_mask));
> > if (proto == IPPROTO_ICMP) {
> > + memset(&icmp_spec, 0, sizeof(icmp_spec));
> > + memset(&icmp_mask, 0, sizeof(icmp_mask));
> > icmp_spec.hdr.icmp_type = (uint8_t)ntohs(match->flow.tp_src);
> > icmp_spec.hdr.icmp_code = (uint8_t)ntohs(match->flow.tp_dst);
> >
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
