Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

Hello Valters,

Hmm, this should not happen like this.

I am not even sure why this is trying to do any ioctls for terminals on the output file.

Is this reproducible with any of the other formats, too, or is it only xt_geoip?

-Michael

> On 29 Mar 2022, at 15:07, Valters Jansons <valter.jansons(a)gmail.com> wrote:
> 
> On v0.9.12, I am observing an issue with XTGeoIP exports: the *.iv4 files do not get written.
> 
> The sample command I use to reproduce this on Ubuntu 20.04 is very basic:
> location export --format xt_geoip --directory /usr/share/xt_geoip
> 
> All IPv6 outputs are written successfully, but upon reaching IPv4 it errors with:
> (25, 'Inappropriate ioctl for device')
> 
> A quicker test case is to add argument `--family ipv4` so that IPv6 is hopped over and it tries IPv4 right away.
> 
> With strace, it seems to be:
> openat(AT_FDCWD, "/usr/share/xt_geoip/A1.iv4", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 4
> fstat(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
> ioctl(4, TCGETS, 0x7ffd234c3260)        = -1 ENOTTY (Inappropriate ioctl for device)
> 
> Please let me know if I can provide any further information.
> 
> Best regards,
> Valters Jansons

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Valters Jansons]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On Tue, Mar 29, 2022 at 5:10 PM Michael Tremer
<michael.tremer(a)ipfire.org> wrote:
> Hmm, this should not happen like this.
>
> I am not even sure why this is trying to do any ioctls for terminals on the output file.
>
> Is this reproducible with any of the other formats, too, or is it only xt_geoip?

Indeed, I could not find what seems to cause this, and have not been
able to work around it.

I did not yet try other exporters - I have been focusing on xt_geoip
as we use it for a system's firewall rules.
But considering the IPv6 files for xt_geoip are being written
correctly, it feels to me like others exporters might work. Will
check.

Wanted to bring this to your attention before spending more time on
it, as you maybe were aware.

-Valters

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Valters Jansons]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Tue, Mar 29, 2022 at 5:16 PM Valters Jansons
<valter.jansons(a)gmail.com> wrote:
> On Tue, Mar 29, 2022 at 5:10 PM Michael Tremer
> <michael.tremer(a)ipfire.org> wrote:
> > Is this reproducible with any of the other formats, too, or is it only xt_geoip?
>
> But considering the IPv6 files for xt_geoip are being written
> correctly, it feels to me like others exporters might work. Will
> check.

Indeed, `export --format ipset --directory ~/ipset` works perfectly
fine, and the *v4.ipset files have content.

This seems to be xt_geoip for IPv4 only. Don't see any specific path
difference between IPv6 and IPv4 though, and I doubt that Ubuntu 20.04
has something to do with it, but you never know I suppose.

Will try to dig deeper.

-Valters

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]

Hello,

> On 30 Mar 2022, at 16:59, Valters Jansons <valter.jansons(a)gmail.com> wrote:
> 
> On Wed, Mar 30, 2022 at 6:36 PM Michael Tremer
> <michael.tremer(a)ipfire.org> wrote:
>> 
>> Hello Valters,
>> 
>> Here is the fix:
>> 
>>  https://git.ipfire.org/?p=location/libloc.git;a=commitdiff;h=47b55e7060f6714889d2a8a45dd01e3452e2db38
>> 
>> The error was shown incorrectly because this was the last syscall that failed (but it was executed by Python and not us).
>> 
>> The real reason is that the wrong prefix was used for IPv4 addresses which resulted in a sanity check failing. However, I did not catch that very well because it is incredibly unlikely to fail.
>> 
>> Please confirm to me whether or not this patch resolves the problem.
> 
> Michael, thank you for finding the bug! I will rebuild with that
> commit applied, and try to test later today.
> 
>> I was also going to ask on this list whether people would rely on support for xt_geoip. IPFire has just replaced xt_geoip with ipset and so we don’t need the code any more which is probably why I didn’t catch this problem earlier - I have added a small test though. Writing more tests, better development documentation and returning better error codes in some places is on our TODO list, but we are incredibly lacking time and human resources to allocate to this. Can you help?
> 
> Due to organizational changes, I sadly don't have as much time for
> external projects as I did before, and therefore cannot help out with
> writing test cases. If you make the decision to drop the xt_geoip
> format, we would of course be sad, but should be able to figure it
> out. If that were the case, is there any expectation for database
> backwards compatibility -- on how long the old versions would be able
> to stay operational, and work with newer database files?

I don’t necessarily want to drop it, but since resources are scarce, I don’t see any reason to maintain something that nobody is using. Since you are using it, and because it works, I don’t see any reason to immediately drop it.

The database format isn’t very flexible, so there is a lot of work to do if that was to be changed. We have a mechanism built in for this, but we need a very strong reason to touch this at all. This is where we get the performance from :)

So, no worries that support for xt_geoip will go away. But please keep reporting any problems that you encounter.

Best,
-Michael

> -Valters

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Valters Jansons]

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]

On Wed, Mar 30, 2022 at 6:36 PM Michael Tremer
<michael.tremer(a)ipfire.org> wrote:
>
> Hello Valters,
>
> Here is the fix:
>
>   https://git.ipfire.org/?p=location/libloc.git;a=commitdiff;h=47b55e7060f6714889d2a8a45dd01e3452e2db38
>
> The error was shown incorrectly because this was the last syscall that failed (but it was executed by Python and not us).
>
> The real reason is that the wrong prefix was used for IPv4 addresses which resulted in a sanity check failing. However, I did not catch that very well because it is incredibly unlikely to fail.
>
> Please confirm to me whether or not this patch resolves the problem.

Michael, thank you for finding the bug! I will rebuild with that
commit applied, and try to test later today.

> I was also going to ask on this list whether people would rely on support for xt_geoip. IPFire has just replaced xt_geoip with ipset and so we don’t need the code any more which is probably why I didn’t catch this problem earlier - I have added a small test though. Writing more tests, better development documentation and returning better error codes in some places is on our TODO list, but we are incredibly lacking time and human resources to allocate to this. Can you help?

Due to organizational changes, I sadly don't have as much time for
external projects as I did before, and therefore cannot help out with
writing test cases. If you make the decision to drop the xt_geoip
format, we would of course be sad, but should be able to figure it
out. If that were the case, is there any expectation for database
backwards compatibility -- on how long the old versions would be able
to stay operational, and work with newer database files?

-Valters

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

Hello Valters,

Here is the fix:

  https://git.ipfire.org/?p=location/libloc.git;a=commitdiff;h=47b55e7060f6714889d2a8a45dd01e3452e2db38

The error was shown incorrectly because this was the last syscall that failed (but it was executed by Python and not us).

The real reason is that the wrong prefix was used for IPv4 addresses which resulted in a sanity check failing. However, I did not catch that very well because it is incredibly unlikely to fail.

Please confirm to me whether or not this patch resolves the problem.

I was also going to ask on this list whether people would rely on support for xt_geoip. IPFire has just replaced xt_geoip with ipset and so we don’t need the code any more which is probably why I didn’t catch this problem earlier - I have added a small test though. Writing more tests, better development documentation and returning better error codes in some places is on our TODO list, but we are incredibly lacking time and human resources to allocate to this. Can you help?

-Michael

> On 29 Mar 2022, at 16:03, Michael Tremer <michael.tremer(a)ipfire.org> wrote:
> 
> Hello Valters,
> 
> Yes I can reproduce this on Debian 11 with the latest build from the Debian repository.
> 
> -Michael
> 
>> On 29 Mar 2022, at 15:43, Valters Jansons <valter.jansons(a)gmail.com> wrote:
>> 
>> I am wondering if maybe it has been broken previously but we never picked it up. Say, from ce1e53c "export: Sightly refactor export logic" in some magical case.
>> 
>> Michael, in any case, can you repro what I am seeing on the current release?
>> 
>> -Valters
> 

Re: [0.9.12] export --format xt_geoip: (25, 'Inappropriate ioctl for device')

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Hello Valters,

Yes I can reproduce this on Debian 11 with the latest build from the Debian repository.

-Michael

> On 29 Mar 2022, at 15:43, Valters Jansons <valter.jansons(a)gmail.com> wrote:
> 
> I am wondering if maybe it has been broken previously but we never picked it up. Say, from ce1e53c "export: Sightly refactor export logic" in some magical case.
> 
> Michael, in any case, can you repro what I am seeing on the current release?
> 
> -Valters