> > >> Joe,
> > >>
> > >> can you please make it work with clang? I sent a patch to ml before. It
> > >> might still apply, so you can review it.
> > >> https://travis-ci.org/muvarov/odp/jobs/223572921
> > >>
> > >> the goal is to find good combination of -mcx16 and -latomic flags. And
> > >> we
> >> Joe,
> >>
> >> can you please make it work with clang? I sent a patch to ml before. It
> >> might still apply, so you can review it.
> >> https://travis-ci.org/muvarov/odp/jobs/223572921
> >>
> >> the goal is to find good combination of -mcx16 and -latomic flags. And
> >> we need to test that
> Joe,
>
> can you please make it work with clang? I sent a patch to ml before. It
> might still apply, so you can review it.
> https://travis-ci.org/muvarov/odp/jobs/223572921
>
> the goal is to find good combination of -mcx16 and -latomic flags. And
> we need to test that it still works on arm
Add an example application implementing lock-free IPv4 fragmentation
and reassembly functionality using ODP's packet "concat" and "split".
Signed-off-by: Joe Savage <joe.sav...@arm.com>
Reviewed-and-tested-by: Bill Fischofer
---
(This code contribution is provided un
> > > A change of architecture shouldn't introduce any additional race
> > > conditions or deadlocks, no. If this were the case, then I agree that
> > > this example would be far too fragile to include. Any standards compliant
> > > implementation of "__atomic_compare_exchange_n" will work fine -
> > > > >> You can have arm, arm64, mips64, etc. directories inside
> > > > >> examples/ipfrag.
> > > > >
> > > > > This does not scale in practice. If another example comes along, does
> > > > > that also need to split whatever is determined to be "arch" code into
> > > > > "arch" directories
> > >> You can have arm, arm64, mips64, etc. directories inside
> > >> examples/ipfrag.
> > >
> > > This does not scale in practice. If another example comes along, does
> > > that also need to split whatever is determined to be "arch" code into
> > > "arch" directories within its own folder?
> >
On 12/04/17 14:44, Dmitry Eremin-Solenikov wrote:
> On 12.04.2017 16:37, Joe Savage wrote:
>> On 12/04/17 14:24, Dmitry Eremin-Solenikov wrote:
>>> On 12.04.2017 16:01, Joe Savage wrote:
>>>> I'm not sure I quite follow your comments regarding $(ARCH_DIR) though. A
On 12/04/17 14:24, Dmitry Eremin-Solenikov wrote:
> On 12.04.2017 16:01, Joe Savage wrote:
>> I'm not sure I quite follow your comments regarding $(ARCH_DIR) though. Are
>> there some ODP-defined macros that I can use instead of "__ARM_ARCH" and
>> friends or somethi
On 12/04/17 13:22, Dmitry Eremin-Solenikov wrote:
> On 12.04.2017 14:05, Joe Savage wrote:
>> On 12/04/17 11:32, Maxim Uvarov wrote:
>>> On 12.04.2017 13:15, Joe Savage wrote:
>>>>>>> The problem is that when we discussed this patch on ODP call people
On 12/04/17 13:24, Dmitry Eremin-Solenikov wrote:
> On 10.02.2017 19:06, Joe Savage wrote:
>> +#ifndef ODP_FRAGREASS_PP_ATOMICS_H_
>> +#define ODP_FRAGREASS_PP_ATOMICS_H_
>> +
>> +#include
>> +
>> +#if defined(__arm__) || defined(__aarch64__)
>>
On 12/04/17 11:32, Maxim Uvarov wrote:
> On 12.04.2017 13:15, Joe Savage wrote:
> >>>> The problem is that when we discussed this patch on ODP call people very
> >>>> worry about having 128bit instructions in ODP examples. At least Petri
> >>>> an
> > > The problem is that when we discussed this patch on ODP call people very
> > > worry about having 128bit instructions in ODP examples. At least Petri
> > > and Barry asked if it would be possible to rewrite that with 64 bit
> > > instructions? Some compilers might not support 128 bits and we
On 28/03/17 12:13, Bill Fischofer wrote:
> On Tue, Mar 28, 2017 at 4:10 AM, Ola Liljedahl <ola.liljed...@linaro.org>
> wrote:
>> On 28 March 2017 at 10:41, Joe Savage <joe.sav...@arm.com> wrote:
>>> Hey,
>>>
>>> I just wanted to clarify somethin
Hey,
I just wanted to clarify something about the expected behaviour of
odp_queue_enq. In the following code snippet, is it acceptable for the assert
to fire? (i.e. for a dequeue after a successful enqueue to fail, with only a
single thread of execution)
odp_queue_t queue;
> > The problem is that when we discussed this patch on ODP call people very
> > worry about having 128bit instructions in ODP examples. At least Petri
> > and Barry asked if it would be possible to rewrite that with 64 bit
> > instructions? Some compilers might not support 128 bits and we need to
> The problem is that when we discussed this patch on ODP call people very
> worry about having 128bit instructions in ODP examples. At least Petri
> and Barry asked if it would be possible to rewrite that with 64 bit
> instructions? Some compilers might not support 128 bits and we need to
> test
> ok, it looks like in case of clang we need to pass -mcx16 and no need to
> pass libatomic. I will check if I do that in my patch.
Oops, I just realised that I was wrong here when I said that this looks like
more of an infrastructural problem than a dependency problem. The use of
> I can not compile your code with clang-3.8, gcc works well. But looks
> like clang does not support libatomic so -latomic -mcx16 does not work
> for functions ending on _16"
> ./test.c:(.text+0x61): undefined reference to `__atomic_load_16'
> ./test.c:(.text+0x92): undefined reference to
Add an example application implementing lock-free IPv4 fragmentation
and reassembly functionality using ODP's packet "concat" and "split".
Signed-off-by: Joe Savage <joe.sav...@arm.com>
Reviewed-and-tested-by: Bill Fischofer <bill.fischo...@linaro.org>
---
(Th
> what happens if you add libatomic in configure on your system?
> ./configure CFLAGS="-latomic"
>
>
> does it fail that there is no such lib?
That works fine for me. But, on the x86 and ARM machines I have to hand,
everything also works perfectly without this additional flag. This is what I
Thanks, Maxim. Assuming that's the last of the additional comments, I think
I've cleaned everything up ready for v3. That is, besides the potential issue
with libatomic.
> > +/**
> > + * Clean up any stale flows within a fraglist
> > + *
> > + * @param fl Pointer to the shared fraglist to
> >>> +int main(void)
> >>> +{
> >>> + odp_instance_t instance;
> >>> + odp_pool_t fragment_pool;
> >>> + odp_shm_t shm;
> >>> + odp_cpumask_t cpumask;
> >>> + odph_odpthread_t threads[MAX_WORKERS] = {};
> >>> + odp_packet_t dequeued_pkts[NUM_PACKETS];
> >>> + int i;
>
Thanks, Maxim. A few comments on some of the feedback so far.
> > +int main(void)
> > +{
> > + odp_instance_t instance;
> > + odp_pool_t fragment_pool;
> > + odp_shm_t shm;
> > + odp_cpumask_t cpumask;
> > + odph_odpthread_t threads[MAX_WORKERS] = {};
> > + odp_packet_t
> we use autotools and check to add or not add cflags or ldflags should be
> there.
> I think in that case it's better to make such check in that file:
> ./platform/linux-generic/m4/configure.m4
>
> So check should be code compilation with libatomic and without and set or
> not set it in ldflags.
> No, only which I wrote before. Main comment is that we need it compile in our
> CI env,
> so you need to add atomic library as Ola wrote. And few places where
> variables need go
> on top.
Okay then. There were some unresolved issues in the email to which I replied
to prompt you, but if you
Any further thoughts on these issues, Maxim? I'm keen to get this patch
merged already!
> > > I raised question about coding style question on today’s arch call
> > > discussion. And agreement was:
> > >
> > > 1. variables are on top. (actually we discussed that but looks like
> > > forget to
> > I raised question about coding style question on today’s arch call
> > discussion. And agreement was:
> >
> > 1. variables are on top. (actually we discussed that but looks like
> > forget to document.) Some exceptions acceptable if you link to 3-rd
> > party code which you can not modify.
Hey Maxim,
I'm adding the mailing list to the CCs.
> > +#include
> > +#include
> > +
> > +#include "odp_ipfragreass_ip.h"
> > +#include "odp_ipfragreass_fragment.h"
> > +
> > +int fragment_ipv4_packet(odp_packet_t orig_packet, odp_packet_t *out,
> > +int *out_len)
> >
> > Checkpatch still has some issues with this:
> >
> > ill at Ubuntu15:~/linaro/review$ ./scripts/checkpatch.pl
> > 0001-example-add-IPv4-fragmentation-reassembly-example.patch
> > WARNING: 'DONT' may be misspelled - perhaps 'DON'T'?
> > #1056: FILE: example/ipfragreass/odp_ipfragreass_ip.h:20:
>
Add an example application implementing lock-free IPv4 fragmentation
and reassembly functionality using ODP's packet "concat" and "split".
Signed-off-by: Joe Savage <joe.sav...@arm.com>
---
(This code contribution is provided under the terms of agreement LES-LTM-21309)
> > we used MAX_WORKERS before odp_cpumask_default_worker() was implemented,
> > after that we did not rewrite old example. I think it's better to have
> > less defines in code.
>
> Ok, will do.
Actually, scratch that. MAX_WORKERS is also used allocate the correct number
of elements in the
> >> It will be very helpful if rehe was some README with description about
> >> this app, run environments and some output. So people can learn
> >> something before looking to code.
> >
> > I can add one, but I don't think there's really that much to describe. Since
> > the example doesn't
> > + return __atomic_compare_exchange_16(var, exp, neu, false,
> > mo_success,
> > + mo_failure);
>
> This is the line that clang complains about. You're probably more
> expert than I in this area but the _odp_atomic_u128_xchg_mm() routine
> that
Thanks, Maxim. Just a few things...
> It will be very helpful if rehe was some README with description about
> this app, run environments and some output. So people can learn
> something before looking to code.
I can add one, but I don't think there's really that much to describe. Since
the
Add an example application implementing lock-free IPv4 fragmentation
and reassembly functionality using ODP's packet "concat" and "split".
Signed-off-by: Joe Savage <joe.sav...@arm.com>
---
(This code contribution is provided under the terms of agreement LES-LTM-21
> Any atomics can be emulated in SW (using compiler built-ins or locks
> directly). The point here is the missing HW support:
> * E.g. MIPS, Power, ARMv7 do not have 128 bit CAS
> * 128 bit fetch-and-add is not supported in any of the architectures
>
> We need to ensure on any operations added
> I wonder what processor supports 128 bits atomics. As far as I know Intel
> does not support it. Lock prefix is not allowed on SSE instructions.
Actually, Intel does support them through a locked cmpxchg16b. And ARMv8
through load exclusive pair and store exclusive pair.
> Unfortunately, ODP atomics API does not support 128 bit atomics - at least
> currently. So, your example could not use those anyway. Not all 64 CPUs
> have 128 bit atomic instructions.
Yes, this is unfortunate. For the time being I am using GCC's __atomic
operations such as
> Agree with Maxim. I which way the application is not 32 bit compliant?
It uses 128-bit atomics, and so is really designed for execution on 64-bit
machines. It is possible to provide lockless 32-bit support in this case,
though, and I have an implementation that does so. Since the pointer size
> Is it a true very direct example of the concept, or more of a mini
> application ?
>
> We are trying to lift out of examples those things that are more
> complex mini apps to a new apps directory.
> One distinction is the requirement to use external traffic generation
> for example would likely
Hey,
I'm just finalising a patch to add a lock-free IPv4 fragmentation and
reassembly example to ODP, and it has occurred to me that as the whole
examples directory is compiled with odp-linux by default, I ought to be
careful about supporting a wide variety of platforms (ensuring that my change
Fix a bug where odph_ipv4_csum_{valid, update} would interpret an L3
offset of zero as a sign that the packet has no L3 offset.
Signed-off-by: Joe Savage <joe.sav...@arm.com>
---
helper/include/odp/helper/ip.h | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/
I recently noticed some unexpected behaviour in "odph_ipv4_csum_update" when
manipulating ODP IPv4 packets with an L3 offset of 0 (i.e. no L2 protocol
wrapping the packet data). It seems that the implementation of this function
uses an L3 offset of zero from "odp_packet_l3_offset" to indicate that
> I am happy to work with you and review your contributions if you want to post
> a distinct apps directory and we can clean up from there, moving existing
> things as appropriate.
Sure, that sounds great. Thanks.
> I think one distinction for sample apps might be that they are not easily
>
> One distinction we looked at but did not act on was keeping examples very
> terse as you say and they are linked to the doxygen. Then more elaborate
> examples can be in a directory called apps to contain mini apps.
That's an interesting distinction to make. Having picked up ODP relatively
Thanks, Bill. If I'm understanding you correctly, the conclusion here is that
at this stage it's very difficult to pin down how implementations on the
whole are likely to behave. This being especially true for the reference APIs,
which aren't currently represented within the wider pool of
> ODP specifies functional behavior of it's APIs. Implementations are free to
> choose whatever techniques or other internals they wish to realize this
> behavior. So things like lazy/deferred/batched operations are entirely
> possible as long as the external semantics are matched.
Sure, and
Hey,
I have recently been looking into the suitability of the ODP APIs for the
fragmentation and reassembly of IP packets. Particularly, I've been focused
on the split and concat APIs — using the former to break larger packets into
smaller fragments, and the latter to glue such fragments back
49 matches
Mail list logo
