diff mbox series

[v3,6/9] net/i40e: display Flow Director packet

Message ID 1578572194-594-7-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/i40e: ESP support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Iremonger, Bernard Jan. 9, 2020, 12:16 p.m. UTC 
  call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Qi Zhang Jan. 9, 2020, 12:44 p.m. UTC | #1 
Hi Bernard:

> -----Original Message-----
> From: Iremonger, Bernard <bernard.iremonger@intel.com>
> Sent: Thursday, January 9, 2020 8:17 PM
> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne, Stephen1
> <stephen1.byrne@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: [PATCH v3 6/9] net/i40e: display Flow Director packet
> 
> call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  drivers/net/i40e/i40e_fdir.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 3fa6297..78329d2 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -21,6 +21,7 @@
>  #include <rte_tcp.h>
>  #include <rte_sctp.h>
>  #include <rte_hash_crc.h>
> +#include <rte_hexdump.h>
> 
>  #include "i40e_logs.h"
>  #include "base/i40e_type.h"
> @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> rte_eth_fdir_input *fdir_input,
>  			    fdir_input->flow_type);
>  		return -1;
>  	}
> +	rte_hexdump(stdout, NULL, raw_pkt, len);

Why we need this? Does this just for debug?

Regards
Qi

>  	return len;
>  }
> 
> @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
>  				 &fdir_input->flow_ext.flexbytes[dst],
>  				 size * sizeof(uint16_t));
>  	}
> -
> +	rte_hexdump(stdout, NULL, raw_pkt, len);
>  	return 0;
>  }
> 
> --
> 2.7.4
Iremonger, Bernard Jan. 9, 2020, 2:02 p.m. UTC | #2 
Hi Qi,


> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Thursday, January 9, 2020 12:44 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org;
> Xing, Beilei <beilei.xing@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne, Stephen1
> <stephen1.byrne@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: RE: [PATCH v3 6/9] net/i40e: display Flow Director packet
> 
> Hi Bernard:
> 
> > -----Original Message-----
> > From: Iremonger, Bernard <bernard.iremonger@intel.com>
> > Sent: Thursday, January 9, 2020 8:17 PM
> > To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne,
> > Stephen1 <stephen1.byrne@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>
> > Subject: [PATCH v3 6/9] net/i40e: display Flow Director packet
> >
> > call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  drivers/net/i40e/i40e_fdir.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_fdir.c
> > b/drivers/net/i40e/i40e_fdir.c index
> > 3fa6297..78329d2 100644
> > --- a/drivers/net/i40e/i40e_fdir.c
> > +++ b/drivers/net/i40e/i40e_fdir.c
> > @@ -21,6 +21,7 @@
> >  #include <rte_tcp.h>
> >  #include <rte_sctp.h>
> >  #include <rte_hash_crc.h>
> > +#include <rte_hexdump.h>
> >
> >  #include "i40e_logs.h"
> >  #include "base/i40e_type.h"
> > @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> > rte_eth_fdir_input *fdir_input,
> >  			    fdir_input->flow_type);
> >  		return -1;
> >  	}
> > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> 
> Why we need this? Does this just for debug?

Useful to see the packet constructed by this function, otherwise no visibility on what is happening.
Needed for debug.

> 
> Regards
> Qi
> 
> >  	return len;
> >  }
> >
> > @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
> >  				 &fdir_input->flow_ext.flexbytes[dst],
> >  				 size * sizeof(uint16_t));
> >  	}
> > -
> > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> >  	return 0;
> >  }
> >
> > --
> > 2.7.4
> 

Regards,

Bernard.
Qi Zhang Jan. 9, 2020, 2:11 p.m. UTC | #3 
> -----Original Message-----
> From: Iremonger, Bernard <bernard.iremonger@intel.com>
> Sent: Thursday, January 9, 2020 10:03 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne, Stephen1
> <stephen1.byrne@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: RE: [PATCH v3 6/9] net/i40e: display Flow Director packet
> 
> Hi Qi,
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Thursday, January 9, 2020 12:44 PM
> > To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org;
> > Xing, Beilei <beilei.xing@intel.com>; Doherty, Declan
> > <declan.doherty@intel.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne,
> > Stephen1 <stephen1.byrne@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>
> > Subject: RE: [PATCH v3 6/9] net/i40e: display Flow Director packet
> >
> > Hi Bernard:
> >
> > > -----Original Message-----
> > > From: Iremonger, Bernard <bernard.iremonger@intel.com>
> > > Sent: Thursday, January 9, 2020 8:17 PM
> > > To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> > > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne,
> > > Stephen1 <stephen1.byrne@intel.com>; Zhang, Helin
> > > <helin.zhang@intel.com>; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>
> > > Subject: [PATCH v3 6/9] net/i40e: display Flow Director packet
> > >
> > > call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_fdir.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_fdir.c
> > > b/drivers/net/i40e/i40e_fdir.c index
> > > 3fa6297..78329d2 100644
> > > --- a/drivers/net/i40e/i40e_fdir.c
> > > +++ b/drivers/net/i40e/i40e_fdir.c
> > > @@ -21,6 +21,7 @@
> > >  #include <rte_tcp.h>
> > >  #include <rte_sctp.h>
> > >  #include <rte_hash_crc.h>
> > > +#include <rte_hexdump.h>
> > >
> > >  #include "i40e_logs.h"
> > >  #include "base/i40e_type.h"
> > > @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> > > rte_eth_fdir_input *fdir_input,
> > >      fdir_input->flow_type);
> > >  return -1;
> > >  }
> > > +rte_hexdump(stdout, NULL, raw_pkt, len);
> >
> > Why we need this? Does this just for debug?
> 
> Useful to see the packet constructed by this function, otherwise no visibility on
> what is happening.
> Needed for debug.

But this may flush the screen if we create 1000 rules by script and it impact the rule programming performance, should this code only be called when debug mode is enabled?
> 
> >
> > Regards
> > Qi
> >
> > >  return len;
> > >  }
> > >
> > > @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
> > >   &fdir_input->flow_ext.flexbytes[dst],
> > >   size * sizeof(uint16_t));
> > >  }
> > > -
> > > +rte_hexdump(stdout, NULL, raw_pkt, len);
> > >  return 0;
> > >  }
> > >
> > > --
> > > 2.7.4
> >
> 
> Regards,
> 
> Bernard.
>
Iremonger, Bernard Jan. 9, 2020, 2:30 p.m. UTC | #4 
Hi Qi,

<snip>

> > > Subject: RE: [PATCH v3 6/9] net/i40e: display Flow Director packet
> > >
> > > Hi Bernard:
> > >
> > > > -----Original Message-----
> > > > From: Iremonger, Bernard <bernard.iremonger@intel.com>
> > > > Sent: Thursday, January 9, 2020 8:17 PM
> > > > To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi
> > > > Z <qi.z.zhang@intel.com>; Doherty, Declan
> > > > <declan.doherty@intel.com>
> > > > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne,
> > > > Stephen1 <stephen1.byrne@intel.com>; Zhang, Helin
> > > > <helin.zhang@intel.com>; Iremonger, Bernard
> > > > <bernard.iremonger@intel.com>
> > > > Subject: [PATCH v3 6/9] net/i40e: display Flow Director packet
> > > >
> > > > call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> > > >
> > > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_fdir.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_fdir.c
> > > > b/drivers/net/i40e/i40e_fdir.c index
> > > > 3fa6297..78329d2 100644
> > > > --- a/drivers/net/i40e/i40e_fdir.c
> > > > +++ b/drivers/net/i40e/i40e_fdir.c
> > > > @@ -21,6 +21,7 @@
> > > >  #include <rte_tcp.h>
> > > >  #include <rte_sctp.h>
> > > >  #include <rte_hash_crc.h>
> > > > +#include <rte_hexdump.h>
> > > >
> > > >  #include "i40e_logs.h"
> > > >  #include "base/i40e_type.h"
> > > > @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> > > > rte_eth_fdir_input *fdir_input,
> > > >      fdir_input->flow_type);
> > > >  return -1;
> > > >  }
> > > > +rte_hexdump(stdout, NULL, raw_pkt, len);
> > >
> > > Why we need this? Does this just for debug?
> >
> > Useful to see the packet constructed by this function, otherwise no
> > visibility on what is happening.
> > Needed for debug.
> 
> But this may flush the screen if we create 1000 rules by script and it impact
> the rule programming performance, should this code only be called when
> debug mode is enabled?

Yes, probably better to call in debug mode only.
Will change in v4 patchset.

> > >
> > > Regards
> > > Qi
> > >
> > > >  return len;
> > > >  }
> > > >
> > > > @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
> > > >   &fdir_input->flow_ext.flexbytes[dst],
> > > >   size * sizeof(uint16_t));
> > > >  }
> > > > -
> > > > +rte_hexdump(stdout, NULL, raw_pkt, len);
> > > >  return 0;
> > > >  }
> > > >
> > > > --
> > > > 2.7.4
> > >
> >
 Regards,

 Bernard.
Ananyev, Konstantin Jan. 9, 2020, 11:07 p.m. UTC | #5 
> > >
> > > call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_fdir.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_fdir.c
> > > b/drivers/net/i40e/i40e_fdir.c index
> > > 3fa6297..78329d2 100644
> > > --- a/drivers/net/i40e/i40e_fdir.c
> > > +++ b/drivers/net/i40e/i40e_fdir.c
> > > @@ -21,6 +21,7 @@
> > >  #include <rte_tcp.h>
> > >  #include <rte_sctp.h>
> > >  #include <rte_hash_crc.h>
> > > +#include <rte_hexdump.h>
> > >
> > >  #include "i40e_logs.h"
> > >  #include "base/i40e_type.h"
> > > @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> > > rte_eth_fdir_input *fdir_input,
> > >  			    fdir_input->flow_type);
> > >  		return -1;
> > >  	}
> > > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> >
> > Why we need this? Does this just for debug?
> 
> Useful to see the packet constructed by this function, otherwise no visibility on what is happening.
> Needed for debug.

Then it probably should be called only when some debug option is enabled
(either at complie or runtime)? 

> 
> >
> > Regards
> > Qi
> >
> > >  	return len;
> > >  }
> > >
> > > @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
> > >  				 &fdir_input->flow_ext.flexbytes[dst],
> > >  				 size * sizeof(uint16_t));
> > >  	}
> > > -
> > > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> > >  	return 0;
> > >  }
> > >
> > > --
> > > 2.7.4
> >
> 
> Regards,
> 
> Bernard.
Iremonger, Bernard Jan. 10, 2020, 9:20 a.m. UTC | #6 
Hi Konstantin,

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, January 9, 2020 11:08 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> Cc: Byrne, Stephen1 <stephen1.byrne@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>
> Subject: RE: [PATCH v3 6/9] net/i40e: display Flow Director packet
> 
> 
> 
> > > >
> > > > call rte_hexdump in i40e_flow_fdir_construct_pkt() in i40e_fdir.c
> > > >
> > > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_fdir.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_fdir.c
> > > > b/drivers/net/i40e/i40e_fdir.c index
> > > > 3fa6297..78329d2 100644
> > > > --- a/drivers/net/i40e/i40e_fdir.c
> > > > +++ b/drivers/net/i40e/i40e_fdir.c
> > > > @@ -21,6 +21,7 @@
> > > >  #include <rte_tcp.h>
> > > >  #include <rte_sctp.h>
> > > >  #include <rte_hash_crc.h>
> > > > +#include <rte_hexdump.h>
> > > >
> > > >  #include "i40e_logs.h"
> > > >  #include "base/i40e_type.h"
> > > > @@ -805,6 +806,7 @@ i40e_fdir_fill_eth_ip_head(const struct
> > > > rte_eth_fdir_input *fdir_input,
> > > >  			    fdir_input->flow_type);
> > > >  		return -1;
> > > >  	}
> > > > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> > >
> > > Why we need this? Does this just for debug?
> >
> > Useful to see the packet constructed by this function, otherwise no
> visibility on what is happening.
> > Needed for debug.
> 
> Then it probably should be called only when some debug option is enabled
> (either at complie or runtime)?

Responded already to Qi that I will call in debug mode in v4 patchset.
I think compile time should be enough.

> 
> >
> > >
> > > Regards
> > > Qi
> > >
> > > >  	return len;
> > > >  }
> > > >
> > > > @@ -954,7 +956,7 @@ i40e_fdir_construct_pkt(struct i40e_pf *pf,
> > > >  				 &fdir_input->flow_ext.flexbytes[dst],
> > > >  				 size * sizeof(uint16_t));
> > > >  	}
> > > > -
> > > > +	rte_hexdump(stdout, NULL, raw_pkt, len);
> > > >  	return 0;
> > > >  }
> > > >
> > > > --
> > > > 2.7.4
> > >
> >
Regards,

Bernard.
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 3fa6297..78329d2 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -21,6 +21,7 @@ 
 #include <rte_tcp.h>
 #include <rte_sctp.h>
 #include <rte_hash_crc.h>
+#include <rte_hexdump.h>
 
 #include "i40e_logs.h"
 #include "base/i40e_type.h"
@@ -805,6 +806,7 @@  i40e_fdir_fill_eth_ip_head(const struct rte_eth_fdir_input *fdir_input,
 			    fdir_input->flow_type);
 		return -1;
 	}
+	rte_hexdump(stdout, NULL, raw_pkt, len);
 	return len;
 }
 
@@ -954,7 +956,7 @@  i40e_fdir_construct_pkt(struct i40e_pf *pf,
 				 &fdir_input->flow_ext.flexbytes[dst],
 				 size * sizeof(uint16_t));
 	}
-
+	rte_hexdump(stdout, NULL, raw_pkt, len);
 	return 0;
 }