On Fri, May 25, 2018 at 05:45:03AM +0000, Fu, Qiaobin wrote:
> Hi Marcelo,
> 
> Thanks for pointing out these style issues. Below is the updated version:

Hi Qiaobin,

Looks good to me. Now you have to submit it like you submitted the
original patch, but add the version tag to the summary. Like '[PATCH
v2 net-next] ....'
And without the text before the changelog.

Thanks.

> 
> ---
> The new action inheritdsfield copies the field DS of
> IPv4 and IPv6 packets into skb->priority. This enables
> later classification of packets based on the DS field.
> 
> Original idea by Jamal Hadi Salim <j...@mojatatu.com>
> 
> Signed-off-by: Qiaobin Fu <qiaob...@bu.edu>
> Reviewed-by: Michel Machado <mic...@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..e2082f6 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,26 @@ 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);
> +
> +             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 +150,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;
> 
> > On May 23, 2018, at 2:06 PM, Marcelo Ricardo Leitner 
> > <marcelo.leit...@gmail.com> wrote:
> > 
> > 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 <j...@mojatatu.com>
> >> 
> >> Signed-off-by: Qiaobin Fu <qiaob...@bu.edu>
> >> Reviewed-by: Michel Machado <mic...@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;
> 

Reply via email to