From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Chan Subject: Re: [PATCH net v2 2/6] bnxt_en: do not allow wildcard matches for L2 flows Date: Wed, 11 Apr 2018 13:55:11 -0700 Message-ID: References: <1523461818-15774-1-git-send-email-michael.chan@broadcom.com> <1523461818-15774-3-git-send-email-michael.chan@broadcom.com> <20180411114303.6f927c45@cakuba.netronome.com> <20180411203152.GD33938@C02RW35GFVH8.dhcp.broadcom.net> <20180411205014.GE33938@C02RW35GFVH8.dhcp.broadcom.net> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Jakub Kicinski , David Miller , Netdev To: Andy Gospodarek Return-path: Received: from mail-oi0-f67.google.com ([209.85.218.67]:43016 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755561AbeDKUzM (ORCPT ); Wed, 11 Apr 2018 16:55:12 -0400 Received: by mail-oi0-f67.google.com with SMTP id u84-v6so3073460oie.10 for ; Wed, 11 Apr 2018 13:55:11 -0700 (PDT) In-Reply-To: <20180411205014.GE33938@C02RW35GFVH8.dhcp.broadcom.net> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Apr 11, 2018 at 1:50 PM, Andy Gospodarek wrote: > On Wed, Apr 11, 2018 at 01:41:31PM -0700, Michael Chan wrote: >> On Wed, Apr 11, 2018 at 1:31 PM, Andy Gospodarek >> wrote: >> > On Wed, Apr 11, 2018 at 11:43:14AM -0700, Jakub Kicinski wrote: >> >> On Wed, 11 Apr 2018 11:50:14 -0400, Michael Chan wrote: >> >> > @@ -764,6 +788,41 @@ static bool bnxt_tc_can_offload(struct bnxt *bp, struct bnxt_tc_flow *flow) >> >> > return false; >> >> > } >> >> > >> >> > + /* Currently source/dest MAC cannot be partial wildcard */ >> >> > + if (bits_set(&flow->l2_key.smac, sizeof(flow->l2_key.smac)) && >> >> > + !is_exactmatch(flow->l2_mask.smac, sizeof(flow->l2_mask.smac))) { >> >> > + netdev_info(bp->dev, "Wildcard match unsupported for Source MAC\n"); >> >> >> >> This wouldn't be something to do in net, but how do you feel about >> >> using extack for messages like this? >> >> >> > >> > I agree 'net' would not have been the place for a change like that, but >> > I do think that would be a good idea. It looks like we could easily >> > change the ndo_setup_tc to something like this: >> > >> > int (*ndo_setup_tc)(struct net_device *dev, >> > enum tc_setup_type type, >> > void *type_data, >> > struct netlink_ext_ack *extack); >> >> I think the extack pointer is already in the tc_cls_common_offload >> struct inside tc_cls_flower_offload struct. > > True, but I'm not sure that tc_cls_common_offload is used in all cases. > Take red_offload() as one of those. For Flower, we know we have the extack pointer in tc_cls_common_offload struct and we can use it to set the netlink error message. The point is that we don't have to modify ndo_setup_tc().