I've been noodling with the old 'assigned virtio' code, trying to make
it work again so I can use this method to get raw packets. See
discusion on the 'raw sockets' thread.
I'm mostly there (though I will admit, I'm far from a C++ programmer,
so the code is pretty hideous), but I ran into an issue with the
interrupt routine crashing. It turns out that if the interrupt routine
is in my .so file, then the symbol resolution is not done at load time,
rather its done on first call.
I was hitting an assert in resolve_pltgot. I remember having a similar
issue a while ago (2016!) - back then it was a bug in
pthread_spin_lock. In this case, because the symbols aren't resolved on
loading of the .so file, the first time the isr is called, we try to
solve the missing symbols. However, we are not sched::preemptable() at
that point, so the assert fires.
I tried the existing isr code, copying it into my code. The wait part
looks like this:
void receiver(virtio::vring *vq)
{
while (1) {
sched::thread::wait_until([&] {
bool have_elements = vq->used_ring_not_empty();
if (!have_elements) {
vq->enable_interrupts();
// we must check that the ring is not empty *after*
// we enable interrupts to avoid a race where a packet
// may have been delivered between queue->used_ring_not_empty()
// and queue->enable_interrupts() above
have_elements = vq->used_ring_not_empty();
if (have_elements) {
vq->disable_interrupts();
}
}
return have_elements;
});
...
sched::thread::wait_until and sched::thread::stop_wait are not
resolved. I changed resolve_pltgot to what was suggested back in 2016:
diff --git a/core/elf.cc b/core/elf.cc
index ffb16004..5cdf1ec2 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -816,7 +816,7 @@ void object::relocate_pltgot()
void* object::resolve_pltgot(unsigned index)
{
- assert(sched::preemptable());
+ // assert(sched::preemptable());
auto rel = dynamic_ptr<Elf64_Rela>(DT_JMPREL);
auto slot = rel[index];
auto info = slot.r_info;
@@ -826,6 +826,14 @@ void* object::resolve_pltgot(unsigned index)
void *addr = _base + slot.r_offset;
auto sm = symbol(sym);
+ if (!sched::preemptable()) {
+ auto nameidx = (dynamic_ptr<Elf64_Sym>(DT_SYMTAB) + sym)-
>st_name;
+ auto name = dynamic_ptr<const char>(DT_STRTAB) + nameidx;
+ std::cerr << "resolve_pltgot " << demangle(name) <<
+ " in " << pathname() << " found in '" <<
+ sm.obj->pathname() << "'\n";
+ }
+
if (sm.obj != this) {
WITH_LOCK(_used_by_resolve_plt_got_mutex) {
_used_by_resolve_plt_got.insert(sm.obj-
>shared_from_this());
This no longer asserts, and shows the symbols being resolved:
resolve_pltgot _ZN5sched6thread9stop_waitEv
(sched::thread::stop_wait()) in /tests/tst-assign-virtio.so found in ''
Handling packets
resolve_pltgot _ZN6virtio5vring17enable_interruptsEv
(virtio::vring::enable_interrupts()) in /tests/tst-assign-virtio.so
found in ''
resolve_pltgot _ZN5sched6thread4waitEv (sched::thread::wait()) in
/tests/tst-assign-virtio.so found in ''
So what is the appropriate solution here? I could try and arrange for
my code to be linked with "-Wl,-z,now" but that feels like quite a big
hammer (as that could then apply to the whole app?)
OTOH, it feels unnatural to be relying on symbol lookups in an isr. If
resolve_pltgot really does not need the assert check (whch was
suggested to be the case in 2016), maybe there is nothing to do here
other than comment out an assert?
Probably I should arrange the code so that I can call
virtio_driver::wait_for_queue, but at the moment, I can't. I don't
think we should assume that the user's code would want that - or do we?
Thoughts?
Rick
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/7fa151d59007a824119e83d6906f64ea1f94fe3d.camel%40rossfell.co.uk.
