From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: Re: [PATCH net-next] net:sched: add action inheritdsfield to skbmod
Date: Wed, 23 May 2018 18:06:28 -0300
Message-ID: <20180523210628.GK5488@localhost.localdomain>
References: <2F042100-2BAC-48E5-887C-5D426B1D5A5B@bu.edu>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "jhs@mojatatu.com" <jhs@mojatatu.com>,
        Michel Machado <michel@digirati.com.br>
To: "Fu, Qiaobin" <qiaobinf@bu.edu>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-qt0-f193.google.com ([209.85.216.193]:34660 "EHLO
        mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S934230AbeEWVGe (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 23 May 2018 17:06:34 -0400
Received: by mail-qt0-f193.google.com with SMTP id m5-v6so30029649qti.1
        for <netdev@vger.kernel.org>; Wed, 23 May 2018 14:06:34 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <2F042100-2BAC-48E5-887C-5D426B1D5A5B@bu.edu>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi,

Some style fixes:

On Thu, May 17, 2018 at 07:33:08PM +0000, Fu, Qiaobin wrote:
> net/sched: add action inheritdsfield to skbmod

This extra line above should not be here.

> 
> The new action inheritdsfield copies the field DS of
> IPv4 and IPv6 packets into skb->prioriry. This enables
                              typo -----^

> later classification of packets based on the DS field.
> 
> Original idea by Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Signed-off-by: Qiaobin Fu <qiaobinf@bu.edu>
> Reviewed-by: Michel Machado <michel@digirati.com.br>
> ---
> 
> Note that the motivation for this patch is found in the following discussion:
> https://www.spinics.net/lists/netdev/msg501061.html
> ---
> 
> diff --git a/include/uapi/linux/tc_act/tc_skbmod.h b/include/uapi/linux/tc_act/tc_skbmod.h
> index 38c072f..0718b48 100644
> --- a/include/uapi/linux/tc_act/tc_skbmod.h
> +++ b/include/uapi/linux/tc_act/tc_skbmod.h
> @@ -19,6 +19,7 @@
>  #define SKBMOD_F_SMAC	0x2
>  #define SKBMOD_F_ETYPE	0x4
>  #define SKBMOD_F_SWAPMAC 0x8
> +#define SKBMOD_F_INHERITDSFIELD 0x10
>  
>  struct tc_skbmod {
>  	tc_gen;
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index ad050d7..21d5bec 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -16,6 +16,9 @@
>  #include <linux/rtnetlink.h>
>  #include <net/netlink.h>
>  #include <net/pkt_sched.h>
> +#include <net/ip.h>
> +#include <net/ipv6.h>
> +#include <net/dsfield.h>
>  
>  #include <linux/tc_act/tc_skbmod.h>
>  #include <net/tc_act/tc_skbmod.h>
> @@ -72,6 +75,25 @@ static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
>  		ether_addr_copy(eth_hdr(skb)->h_source, (u8 *)tmpaddr);
>  	}
>  
> +	if (flags & SKBMOD_F_INHERITDSFIELD) {
> +		int wlen = skb_network_offset(skb);

You need a blank line here, between var declaration and the rest.

> +		switch (tc_skb_protocol(skb)) {
> +		case htons(ETH_P_IP):
> +			wlen += sizeof(struct iphdr);
> +			if (!pskb_may_pull(skb, wlen))
> +				return TC_ACT_SHOT;
> +			skb->priority = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
> +			break;
> +
> +		case htons(ETH_P_IPV6):
> +			wlen += sizeof(struct ipv6hdr);
> +			if (!pskb_may_pull(skb, wlen))
> +				return TC_ACT_SHOT;
> +			skb->priority = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;
> +			break;
> +		}
> +	}
> +
>  	return action;
>  }
>  
> @@ -127,6 +149,9 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
>  	if (parm->flags & SKBMOD_F_SWAPMAC)
>  		lflags = SKBMOD_F_SWAPMAC;
>  
> +	if (parm->flags & SKBMOD_F_INHERITDSFIELD)
> +		lflags |= SKBMOD_F_INHERITDSFIELD;
> +
>  	exists = tcf_idr_check(tn, parm->index, a, bind);
>  	if (exists && bind)
>  		return 0;