From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamal Hadi Salim Subject: Re: [PATCH net-next 1/1] net sched actions: mirred add support for setting Dst MAC address Date: Sat, 2 Jul 2016 10:02:30 -0400 Message-ID: <5777C976.4000402@mojatatu.com> References: <1467465978-28271-1-git-send-email-jhs@emojatatu.com> <5777C673.8000703@cumulusnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, xiyou.wangcong@gmail.com, daniel@iogearbox.net To: Nikolay Aleksandrov , davem@davemloft.net Return-path: Received: from mail-it0-f66.google.com ([209.85.214.66]:33218 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751360AbcGBOCd (ORCPT ); Sat, 2 Jul 2016 10:02:33 -0400 Received: by mail-it0-f66.google.com with SMTP id y93so4945051ita.0 for ; Sat, 02 Jul 2016 07:02:33 -0700 (PDT) In-Reply-To: <5777C673.8000703@cumulusnetworks.com> Sender: netdev-owner@vger.kernel.org List-ID: On 16-07-02 09:49 AM, Nikolay Aleksandrov wrote: > On 02/07/16 15:26, Jamal Hadi Salim wrote: >> From: Jamal Hadi Salim >> >> Often redirecting or mirroring requires that we set the MAC address >> of the target device. While it is possible to pipe to a pedit action >> this obsoletes the need for that. This is justified feature because >> the dst MAC addresses rewrite is such a common use case. >> >> Sample usage: >> sudo $TC filter add dev $ETH parent 1: protocol ip prio 10 \ >> u32 match ip protocol 1 0xff flowid 1:2 \ >> action mirred egress redirect dev $SPANPORT dst 02:15:15:15:15:15 >> >> This will match all icmp packets going out on dev $ETH and >> redirect them to dev $SPANPORT while setting their dst MAC address >> to 02:15:15:15:15:15 >> >> Signed-off-by: Jamal Hadi Salim >> --- >> include/net/tc_act/tc_mirred.h | 4 +++- >> include/uapi/linux/tc_act/tc_mirred.h | 7 ++++--- >> net/sched/act_mirred.c | 20 +++++++++++++++++++- >> 3 files changed, 26 insertions(+), 5 deletions(-) >> >> diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h >> index e891835..7e8bced 100644 >> --- a/include/net/tc_act/tc_mirred.h >> +++ b/include/net/tc_act/tc_mirred.h >> @@ -6,10 +6,12 @@ >> >> struct tcf_mirred { >> struct tcf_common common; >> + struct net_device __rcu *tcfm_dev; >> int tcfm_eaction; >> int tcfm_ifindex; >> int tcfm_ok_push; >> - struct net_device __rcu *tcfm_dev; >> + u8 eth_dst[ETH_ALEN]; >> + /* XXX 6 bytes hole here*/ >> struct list_head tcfm_list; >> }; >> #define to_mirred(a) \ >> diff --git a/include/uapi/linux/tc_act/tc_mirred.h b/include/uapi/linux/tc_act/tc_mirred.h >> index 3d7a2b3..aaca1ff 100644 >> --- a/include/uapi/linux/tc_act/tc_mirred.h >> +++ b/include/uapi/linux/tc_act/tc_mirred.h >> @@ -9,20 +9,21 @@ >> #define TCA_EGRESS_MIRROR 2 /* mirror packet to EGRESS */ >> #define TCA_INGRESS_REDIR 3 /* packet redirect to INGRESS*/ >> #define TCA_INGRESS_MIRROR 4 /* mirror packet to INGRESS */ >> - >> + >> struct tc_mirred { >> tc_gen; >> int eaction; /* one of IN/EGRESS_MIRROR/REDIR */ >> __u32 ifindex; /* ifindex of egress port */ >> }; >> - >> + >> enum { >> TCA_MIRRED_UNSPEC, >> TCA_MIRRED_TM, >> TCA_MIRRED_PARMS, >> TCA_MIRRED_PAD, >> + TCA_MIRRED_DMAC, > > Hi Jamal, > I think you should update "mirred_policy" in order to ensure that the attribute has > the minimum length for a mac address. Good point. Will do in the next update. > Also a minor suggestion - maybe err out on a > zero mac address, otherwise the user might think the operation was successful. > Is a zero mac address wrong? What if that was policy intent? cheers, jamal