[PATCH iproute2 0/4] tc: m_ife: handle IFE ethertype value

[PATCH iproute2 0/4] tc: m_ife: handle IFE ethertype value

[Alexander Aring]

Hi,

this patch series fix some ife type handling for example it allows to set
ethertype to zero and prints IFE type like IEE specification.

Also it will report about the new introduced IFE type as fallback if no
IFE type as parameter is given. This new behaviour is also added to the
manpage of tc-ife.

Note: Please wait until the related kernel patches comes in. This
patch series contains an UAPI change which need to be accepted by netdev
at first. Thanks.

- Alex

Alexander Aring (4):
  tc: m_ife: allow ife type to zero
  tc: m_ife: print IEEE ethertype format
  tc: m_ife: report about kernels default type
  man: tc-ife: add default type note

 man/man8/tc-ife.8 | 2 +-
 tc/m_ife.c        | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH iproute2 1/4] tc: m_ife: allow ife type to zero

[Alexander Aring]

This patch allows to set an ethertype for IFE which is zero. There is no
kernel side validation which forbids a type to zero.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 tc/m_ife.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index e3521e62..e05e2276 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -63,6 +63,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 	char dbuf[ETH_ALEN];
 	char sbuf[ETH_ALEN];
 	__u16 ife_type = 0;
+	int user_type = 0;
 	__u32 ife_prio = 0;
 	__u32 ife_prio_v = 0;
 	__u32 ife_mark = 0;
@@ -125,6 +126,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 			if (get_u16(&ife_type, *argv, 0))
 				invarg("ife type is invalid", *argv);
 			fprintf(stderr, "IFE type 0x%x\n", ife_type);
+			user_type = 1;
 		} else if (matches(*argv, "dst") == 0) {
 			NEXT_ARG();
 			daddr = *argv;
@@ -185,7 +187,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 
 	if (daddr)
 		addattr_l(n, MAX_MSG, TCA_IFE_DMAC, dbuf, ETH_ALEN);
-	if (ife_type)
+	if (user_type)
 		addattr_l(n, MAX_MSG, TCA_IFE_TYPE, &ife_type, 2);
 	if (saddr)
 		addattr_l(n, MAX_MSG, TCA_IFE_SMAC, sbuf, ETH_ALEN);
-- 
2.11.0

[PATCH iproute2 2/4] tc: m_ife: print IEEE ethertype format

[Alexander Aring]

This patch uses the usually IEEE format to display an ethertype which is
4-digits and every digit in upper case.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 tc/m_ife.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index e05e2276..7b57130e 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -125,7 +125,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 			NEXT_ARG();
 			if (get_u16(&ife_type, *argv, 0))
 				invarg("ife type is invalid", *argv);
-			fprintf(stderr, "IFE type 0x%x\n", ife_type);
+			fprintf(stderr, "IFE type 0x%04X\n", ife_type);
 			user_type = 1;
 		} else if (matches(*argv, "dst") == 0) {
 			NEXT_ARG();
-- 
2.11.0

[PATCH iproute2 3/4] tc: m_ife: report about kernels default type

[Alexander Aring]

This patch will report about if the ethertype for IFE is not specified
that the default IFE type is used.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 tc/m_ife.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index 7b57130e..5633ab90 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -189,6 +189,8 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 		addattr_l(n, MAX_MSG, TCA_IFE_DMAC, dbuf, ETH_ALEN);
 	if (user_type)
 		addattr_l(n, MAX_MSG, TCA_IFE_TYPE, &ife_type, 2);
+	else
+		fprintf(stderr, "IFE type 0x%04X\n", ETH_P_IFE);
 	if (saddr)
 		addattr_l(n, MAX_MSG, TCA_IFE_SMAC, sbuf, ETH_ALEN);
 
-- 
2.11.0

[PATCH iproute2 4/4] man: tc-ife: add default type note

[Alexander Aring]

This patch updates the tc-ife man page that the default IFE ethertype
will be used if it's not specified.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 man/man8/tc-ife.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/tc-ife.8 b/man/man8/tc-ife.8
index 3a42d51b..fd2df6c3 100644
--- a/man/man8/tc-ife.8
+++ b/man/man8/tc-ife.8
@@ -82,7 +82,7 @@ is required only when
 Optional six byte destination or source MAC address to encode.
 .TP
 .BI type " TYPE"
-Optional 16-bit ethertype to encode.
+Optional 16-bit ethertype to encode. If not specified value of 0xED3E will be used.
 .TP
 .BI CONTROL
 Action to take following an encode/decode.
-- 
2.11.0

Re: [PATCH iproute2 2/4] tc: m_ife: print IEEE ethertype format

[Stephen Hemminger]

On Mon, 28 Aug 2017 15:07:36 -0400
Alexander Aring <aring@mojatatu.com> wrote:

> This patch uses the usually IEEE format to display an ethertype which is
> 4-digits and every digit in upper case.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  tc/m_ife.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tc/m_ife.c b/tc/m_ife.c
> index e05e2276..7b57130e 100644
> --- a/tc/m_ife.c
> +++ b/tc/m_ife.c
> @@ -125,7 +125,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
>  			NEXT_ARG();
>  			if (get_u16(&ife_type, *argv, 0))
>  				invarg("ife type is invalid", *argv);
> -			fprintf(stderr, "IFE type 0x%x\n", ife_type);
> +			fprintf(stderr, "IFE type 0x%04X\n", ife_type);
>  			user_type = 1;
>  		} else if (matches(*argv, "dst") == 0) {
>  			NEXT_ARG();

Why upper case?

Re: [PATCH iproute2 1/4] tc: m_ife: allow ife type to zero

[Stephen Hemminger]

On Mon, 28 Aug 2017 15:07:35 -0400
Alexander Aring <aring@mojatatu.com> wrote:

> This patch allows to set an ethertype for IFE which is zero. There is no
> kernel side validation which forbids a type to zero.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  tc/m_ife.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tc/m_ife.c b/tc/m_ife.c
> index e3521e62..e05e2276 100644
> --- a/tc/m_ife.c
> +++ b/tc/m_ife.c
> @@ -63,6 +63,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
>  	char dbuf[ETH_ALEN];
>  	char sbuf[ETH_ALEN];
>  	__u16 ife_type = 0;
> +	int user_type = 0;

Please use bool if it is a flag value

Re: [PATCH iproute2 1/4] tc: m_ife: allow ife type to zero

[Jamal Hadi Salim]

On 17-08-28 03:07 PM, Alexander Aring wrote:
> This patch allows to set an ethertype for IFE which is zero. There is no
> kernel side validation which forbids a type to zero.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [PATCH iproute2 2/4] tc: m_ife: print IEEE ethertype format

[Jamal Hadi Salim]

Alex,

I think we should get rid of these fprintfs instead of fixing them.
They were originally intended to be debug outputs.

cheers,
jamal

On 17-08-28 03:07 PM, Alexander Aring wrote:
> This patch uses the usually IEEE format to display an ethertype which is
> 4-digits and every digit in upper case.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>   tc/m_ife.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tc/m_ife.c b/tc/m_ife.c
> index e05e2276..7b57130e 100644
> --- a/tc/m_ife.c
> +++ b/tc/m_ife.c
> @@ -125,7 +125,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
>   			NEXT_ARG();
>   			if (get_u16(&ife_type, *argv, 0))
>   				invarg("ife type is invalid", *argv);
> -			fprintf(stderr, "IFE type 0x%x\n", ife_type);
> +			fprintf(stderr, "IFE type 0x%04X\n", ife_type);
>   			user_type = 1;
>   		} else if (matches(*argv, "dst") == 0) {
>   			NEXT_ARG();
> 

Re: [PATCH iproute2 3/4] tc: m_ife: report about kernels default type

[Jamal Hadi Salim]

Same comment as previous patch.

cheers,
jamal

On 17-08-28 03:07 PM, Alexander Aring wrote:
> This patch will report about if the ethertype for IFE is not specified
> that the default IFE type is used.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>   tc/m_ife.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tc/m_ife.c b/tc/m_ife.c
> index 7b57130e..5633ab90 100644
> --- a/tc/m_ife.c
> +++ b/tc/m_ife.c
> @@ -189,6 +189,8 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
>   		addattr_l(n, MAX_MSG, TCA_IFE_DMAC, dbuf, ETH_ALEN);
>   	if (user_type)
>   		addattr_l(n, MAX_MSG, TCA_IFE_TYPE, &ife_type, 2);
> +	else
> +		fprintf(stderr, "IFE type 0x%04X\n", ETH_P_IFE);
>   	if (saddr)
>   		addattr_l(n, MAX_MSG, TCA_IFE_SMAC, sbuf, ETH_ALEN);
>   
> 

Re: [PATCH iproute2 4/4] man: tc-ife: add default type note

[Jamal Hadi Salim]

On 17-08-28 03:07 PM, Alexander Aring wrote:
> This patch updates the tc-ife man page that the default IFE ethertype
> will be used if it's not specified.
> 
> Signed-off-by: Alexander Aring <aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [PATCH iproute2 2/4] tc: m_ife: print IEEE ethertype format

[Stephen Hemminger]

On Mon, 28 Aug 2017 18:18:04 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> Alex,
> 
> I think we should get rid of these fprintfs instead of fixing them.
> They were originally intended to be debug outputs.
> 
> cheers,
> jamal
> 
> On 17-08-28 03:07 PM, Alexander Aring wrote:
> > This patch uses the usually IEEE format to display an ethertype which is
> > 4-digits and every digit in upper case.
> > 
> > Signed-off-by: Alexander Aring <aring@mojatatu.com>
> > ---
> >   tc/m_ife.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tc/m_ife.c b/tc/m_ife.c
> > index e05e2276..7b57130e 100644
> > --- a/tc/m_ife.c
> > +++ b/tc/m_ife.c
> > @@ -125,7 +125,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
> >   			NEXT_ARG();
> >   			if (get_u16(&ife_type, *argv, 0))
> >   				invarg("ife type is invalid", *argv);
> > -			fprintf(stderr, "IFE type 0x%x\n", ife_type);
> > +			fprintf(stderr, "IFE type 0x%04X\n", ife_type);
> >   			user_type = 1;
> >   		} else if (matches(*argv, "dst") == 0) {
> >   			NEXT_ARG();
> >   
> 


For iproute commands the show output is supposed to match the corresponding set inputs.

Re: [PATCH iproute2 2/4] tc: m_ife: print IEEE ethertype format

[Jamal Hadi Salim]

On 17-08-28 06:37 PM, Stephen Hemminger wrote:
> On Mon, 28 Aug 2017 18:18:04 -0400

[..]
>>> @@ -125,7 +125,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
>>>    			NEXT_ARG();
>>>    			if (get_u16(&ife_type, *argv, 0))
>>>    				invarg("ife type is invalid", *argv);
>>> -			fprintf(stderr, "IFE type 0x%x\n", ife_type);
>>> +			fprintf(stderr, "IFE type 0x%04X\n", ife_type);
>>>    			user_type = 1;
>>>    		} else if (matches(*argv, "dst") == 0) {
>>>    			NEXT_ARG();
>>>    
>>
> 
> 
> For iproute commands the show output is supposed to match the corresponding set inputs.
> 

These were being printed at parse time ...

cheers,
jamal