OK, got your point. We should not reject a possible valid fdir flow at n-tuple flow check stage.
Review-by: Qi Zhang <[email protected]> Thanks Qi From: mocan [mailto:[email protected]] Sent: Wednesday, September 26, 2018 4:16 PM To: Zhang, Qi Z <[email protected]> Cc: [email protected] Subject: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to jump over ntuple filter case Hi Qi, In ixgbe_flow_create function, ntuple filter is parsed first. If the flow is considered to be ntuple filter, it will not go on to judge ethertype filter, syn filter and fdir filter. In the function ntuple_filter_to_5tuple, 5 tuple info is checked, but it's too late to jump over the ntuple filter if it's a fdir filter. At 2018-09-21 23:48:37, "Zhang, Qi Z" <[email protected]> wrote: >Hi Faicker: > >> -----Original Message----- >> From: dev [mailto:[email protected]] On Behalf Of faicker.mo >> Sent: Tuesday, September 18, 2018 1:49 PM >> To: [email protected] >> Cc: faicker.mo <[email protected]> >> Subject: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to jump over >> ntuple filter case >> >> From: "faicker.mo" <[email protected]> >> >> Check in func ntuple_filter_to_5tuple is too late for fdir filter rule, add >> check >> in func cons_parse_ntuple_filter. > >Would you explain more about the intention for this patch? >Though it can be more fast to reject an invalid flow, but why it is too late >in your case? > >Thanks >Qi > > >> >> Signed-off-by: faicker.mo <[email protected]> >> --- >> drivers/net/ixgbe/ixgbe_flow.c | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c >> index 1adf1b8..f0fafeb 100644 >> --- a/drivers/net/ixgbe/ixgbe_flow.c >> +++ b/drivers/net/ixgbe/ixgbe_flow.c >> @@ -363,6 +363,17 @@ const struct rte_flow_action *next_no_void_action( >> item, "Not supported by ntuple filter"); >> return -rte_errno; >> } >> + if ((ipv4_mask->hdr.src_addr != 0 && >> + ipv4_mask->hdr.src_addr != UINT32_MAX) || >> + (ipv4_mask->hdr.dst_addr != 0 && >> + ipv4_mask->hdr.dst_addr != UINT32_MAX) || >> + (ipv4_mask->hdr.next_proto_id != UINT8_MAX && >> + ipv4_mask->hdr.next_proto_id != 0)) { >> + rte_flow_error_set(error, >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, >> + item, "Not supported by ntuple filter"); >> + return -rte_errno; >> + } >> >> filter->dst_ip_mask = ipv4_mask->hdr.dst_addr; >> filter->src_ip_mask = ipv4_mask->hdr.src_addr; @@ -432,6 +443,15 >> @@ const struct rte_flow_action *next_no_void_action( >> item, "Not supported by ntuple filter"); >> return -rte_errno; >> } >> + if ((tcp_mask->hdr.src_port != 0 && >> + tcp_mask->hdr.src_port != UINT16_MAX) || >> + (tcp_mask->hdr.dst_port != 0 && >> + tcp_mask->hdr.dst_port != UINT16_MAX)) { >> + rte_flow_error_set(error, >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, >> + item, "Not supported by ntuple filter"); >> + return -rte_errno; >> + } >> >> filter->dst_port_mask = tcp_mask->hdr.dst_port; >> filter->src_port_mask = tcp_mask->hdr.src_port; @@ -467,6 >> +487,15 @@ const struct rte_flow_action *next_no_void_action( >> item, "Not supported by ntuple filter"); >> return -rte_errno; >> } >> + if ((udp_mask->hdr.src_port != 0 && >> + udp_mask->hdr.src_port != UINT16_MAX) || >> + (udp_mask->hdr.dst_port != 0 && >> + udp_mask->hdr.dst_port != UINT16_MAX)) { >> + rte_flow_error_set(error, >> + EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, >> + item, "Not supported by ntuple filter"); >> + return -rte_errno; >> + } >> >> filter->dst_port_mask = udp_mask->hdr.dst_port; >> filter->src_port_mask = udp_mask->hdr.src_port; >> -- >> 1.8.3.1 >> >
