| Message ID | 20211124091821.3916046-5-boon.leong.ong@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | samples/bpf: xdpsock app enhancements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 16 of 16 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 16 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
On Wed, Nov 24, 2021 at 1:22 AM Ong Boon Leong <boon.leong.ong@intel.com> wrote: > > When user sets tx-pkt-count and in case where there are invalid Tx frame, > the complete_tx_only_all() process polls indefinitely. So, this patch > adds a time-out mechanism into the process so that the application > can terminate automatically after it retries 3*polling interval duration. > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 136383 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 35 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 957 7011 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7012 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7013 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7014 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 0 7014 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 0.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 0 7014 I am not following why we need examples above in the commit log. > > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com> > --- > samples/bpf/xdpsock_user.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c > index 61d4063f11a..9c3311329ec 100644 > --- a/samples/bpf/xdpsock_user.c > +++ b/samples/bpf/xdpsock_user.c > @@ -1410,6 +1410,7 @@ static inline int get_batch_size(int pkt_cnt) > > static void complete_tx_only_all(void) > { > + u32 retries = 3; Shall we make the retry value configurable? And maybe make it a timeout value in seconds? > bool pending; > int i; > > @@ -1421,7 +1422,8 @@ static void complete_tx_only_all(void) > pending = !!xsks[i]->outstanding_tx; > } > } > - } while (pending); > + sleep(opt_interval); > + } while (pending && retries-- > 0); > } > > static void tx_only_all(void) > -- > 2.25.1 >
On 24/11/2021 10.18, Ong Boon Leong wrote: > When user sets tx-pkt-count and in case where there are invalid Tx frame, > the complete_tx_only_all() process polls indefinitely. So, this patch > adds a time-out mechanism into the process so that the application > can terminate automatically after it retries 3*polling interval duration. > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 136383 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 35 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 957 7011 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7012 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7013 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 1 7014 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 1.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 0 7014 > > sock0@enp0s29f1:2 txonly xdp-drv > pps pkts 0.00 > rx 0 0 > tx 0 1000000 > rx dropped 0 0 > rx invalid 0 0 > tx invalid 0 245 > rx queue full 0 0 > fill ring empty 0 1 > tx ring empty 0 7014 > > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com> > --- > samples/bpf/xdpsock_user.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c > index 61d4063f11a..9c3311329ec 100644 > --- a/samples/bpf/xdpsock_user.c > +++ b/samples/bpf/xdpsock_user.c > @@ -1410,6 +1410,7 @@ static inline int get_batch_size(int pkt_cnt) > > static void complete_tx_only_all(void) > { > + u32 retries = 3; > bool pending; > int i; > > @@ -1421,7 +1422,8 @@ static void complete_tx_only_all(void) > pending = !!xsks[i]->outstanding_tx; > } > } > - } while (pending); > + sleep(opt_interval); Why/how is this connected with the 'opt_interval' ? (Which is used by the pthtread 'poller' dumping stats) > + } while (pending && retries-- > 0); > } > > static void tx_only_all(void) >
>On Wed, Nov 24, 2021 at 1:22 AM Ong Boon Leong ><boon.leong.ong@intel.com> wrote: >> >> When user sets tx-pkt-count and in case where there are invalid Tx frame, >> the complete_tx_only_all() process polls indefinitely. So, this patch >> adds a time-out mechanism into the process so that the application >> can terminate automatically after it retries 3*polling interval duration. >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 1.00 >> rx 0 0 >> tx 136383 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 35 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 957 7011 >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 1.00 >> rx 0 0 >> tx 0 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 0 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 1 7012 >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 1.00 >> rx 0 0 >> tx 0 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 0 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 1 7013 >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 1.00 >> rx 0 0 >> tx 0 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 0 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 1 7014 >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 1.00 >> rx 0 0 >> tx 0 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 0 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 0 7014 >> >> sock0@enp0s29f1:2 txonly xdp-drv >> pps pkts 0.00 >> rx 0 0 >> tx 0 1000000 >> rx dropped 0 0 >> rx invalid 0 0 >> tx invalid 0 245 >> rx queue full 0 0 >> fill ring empty 0 1 >> tx ring empty 0 7014 > >I am not following why we need examples above in the commit log. I pasted the log to demonstrate the behavior of the time-out, that is aligned with the stats poller. Will remove in next rev. > >> >> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com> >> --- >> samples/bpf/xdpsock_user.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c >> index 61d4063f11a..9c3311329ec 100644 >> --- a/samples/bpf/xdpsock_user.c >> +++ b/samples/bpf/xdpsock_user.c >> @@ -1410,6 +1410,7 @@ static inline int get_batch_size(int pkt_cnt) >> >> static void complete_tx_only_all(void) >> { >> + u32 retries = 3; > >Shall we make the retry value configurable? And maybe make it a timeout >value in seconds? OK, next rev will have seconds granularity. Ok, I can make it configurable.
>> static void complete_tx_only_all(void) >> { >> + u32 retries = 3; >> bool pending; >> int i; >> >> @@ -1421,7 +1422,8 @@ static void complete_tx_only_all(void) >> pending = !!xsks[i]->outstanding_tx; >> } >> } >> - } while (pending); >> + sleep(opt_interval); > >Why/how is this connected with the 'opt_interval' ? > >(Which is used by the pthtread 'poller' dumping stats) > The original thought was to use the poller interval since it is what user would experience from terminal. In next rev, I plan to make it configurable with second granularity as suggested by Song Liu. Thanks
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c index 61d4063f11a..9c3311329ec 100644 --- a/samples/bpf/xdpsock_user.c +++ b/samples/bpf/xdpsock_user.c @@ -1410,6 +1410,7 @@ static inline int get_batch_size(int pkt_cnt) static void complete_tx_only_all(void) { + u32 retries = 3; bool pending; int i; @@ -1421,7 +1422,8 @@ static void complete_tx_only_all(void) pending = !!xsks[i]->outstanding_tx; } } - } while (pending); + sleep(opt_interval); + } while (pending && retries-- > 0); } static void tx_only_all(void)
When user sets tx-pkt-count and in case where there are invalid Tx frame, the complete_tx_only_all() process polls indefinitely. So, this patch adds a time-out mechanism into the process so that the application can terminate automatically after it retries 3*polling interval duration. sock0@enp0s29f1:2 txonly xdp-drv pps pkts 1.00 rx 0 0 tx 136383 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 35 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 957 7011 sock0@enp0s29f1:2 txonly xdp-drv pps pkts 1.00 rx 0 0 tx 0 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 0 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 1 7012 sock0@enp0s29f1:2 txonly xdp-drv pps pkts 1.00 rx 0 0 tx 0 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 0 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 1 7013 sock0@enp0s29f1:2 txonly xdp-drv pps pkts 1.00 rx 0 0 tx 0 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 0 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 1 7014 sock0@enp0s29f1:2 txonly xdp-drv pps pkts 1.00 rx 0 0 tx 0 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 0 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 0 7014 sock0@enp0s29f1:2 txonly xdp-drv pps pkts 0.00 rx 0 0 tx 0 1000000 rx dropped 0 0 rx invalid 0 0 tx invalid 0 245 rx queue full 0 0 fill ring empty 0 1 tx ring empty 0 7014 Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com> --- samples/bpf/xdpsock_user.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)