On 02/07/16 16:02, Jamal Hadi Salim wrote:
> On 16-07-02 09:49 AM, Nikolay Aleksandrov wrote:
>> On 02/07/16 15:26, Jamal Hadi Salim wrote:
>>> From: Jamal Hadi Salim <j...@mojatatu.com>
>>>
>>> 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 <j...@mojatatu.com>
>>> ---
>>> 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?
>
If you mean that you give the user ability to get rid of the mac, then okay. I
said it
because it will seem like a successful operation and then the mac will not be
dumped
or overwritten which will look like it wasn't set at all.
> cheers,
> jamal
>
