On Mon, Nov 19, 2018 at 05:00:13PM +0100, Jiri Pirko wrote:
> Mon, Nov 19, 2018 at 01:15:19AM CET, pa...@netfilter.org wrote:
[...]
> >-static int qede_flow_spec_to_tuple(struct qede_dev *edev,
> >- struct qede_arfs_tuple *t,
> >- struct ethtool_rx_flow_spec *fs)
> >+static int qede_flow_spec_to_rule(struct qede_dev *edev,
> >+ struct qede_arfs_tuple *t,
> >+ struct ethtool_rx_flow_spec *fs)
> > {
> >- memset(t, 0, sizeof(*t));
> >-
> >- if (qede_flow_spec_validate_unused(edev, fs))
> >- return -EOPNOTSUPP;
> >+ struct tc_cls_flower_offload f = {};
> >+ struct flow_rule *flow_rule;
> >+ __be16 proto;
> >+ int err = 0;
> >
> > switch ((fs->flow_type & ~FLOW_EXT)) {
> > case TCP_V4_FLOW:
> >- return qede_flow_spec_to_tuple_tcpv4(edev, t, fs);
> > case UDP_V4_FLOW:
> >- return qede_flow_spec_to_tuple_udpv4(edev, t, fs);
> >+ proto = htons(ETH_P_IP);
> >+ break;
> > case TCP_V6_FLOW:
> >- return qede_flow_spec_to_tuple_tcpv6(edev, t, fs);
> > case UDP_V6_FLOW:
> >- return qede_flow_spec_to_tuple_udpv6(edev, t, fs);
> >+ proto = htons(ETH_P_IPV6);
> >+ break;
> > default:
> > DP_VERBOSE(edev, NETIF_MSG_IFUP,
> > "Can't support flow of type %08x\n", fs->flow_type);
> > return -EOPNOTSUPP;
> > }
> >
> >- return 0;
> >+ flow_rule = ethtool_rx_flow_rule(fs);
> >+ if (!flow_rule)
> >+ return -ENOMEM;
> >+
> >+ f.rule = *flow_rule;
>
> This does not look right. I undersntand that you want to use the same
> driver code to parse same struct coming either from tc-flower or
> ethtool. That is why you introduced flow_rule as a intermediate layer.
> However, here, you use struct that is very tc-flower specific. Common
> parser should work on struct flow_rule.
>
> qede_parse_flower_attr() should accept struct flow_rule * and should be
> renamed to something like qede_parse_flow_rule().
That was intentional, to keep the number of changes in this drivers as
small as possible while introducing the flow_rule infrastructure.
I'll rework the driver as you're asking, so I can pass struct
flow_rule instead to all parser functions.
The patchset is already rather large, and involving many driver, I was
trying to keep changes to minimal but I'll update this driver as you
request, no problem.
Thanks.
