[devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Arseny Maslennikov]

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

On Thu, May 24, 2018 at 03:14:39AM +0300, Leonid Krivoshein wrote:
> По предыдущему опыту желающих тестировать и ревьювить такое мало, если
> не сказать "нет совсем".

Я готов содействовать по мере наличия времени.
Нам в ACL пропагатора (пока его ещё не закопали) нужен кто-то, кто готов
мониторить возможность сетевой загрузки дистрибутивов, без этого ошибки
исправляться не будут. Насколько мне известно, текущий состав ACL просто
сильно загружен.

https://bugzilla.altlinux.org/34347
https://bugzilla.altlinux.org/34320

Пользуясь случаем, всё-таки прошу одобрить таск 206842, исправляющий
баги 34347 и 34320. Если код слишком плохой даже для пропагатора,
я поправлю, мне не жалко. :)

> ...такие вещи обсуждать лучше в devel@.

Тогда пойдёмте в devel@!

> Например, вот эту цитату:
> 
> Servers that respond SHOULD only use option 43 to return the vendor-specific
> information to the client.
> из RFC-2132 9.13 трактуют по-разному, вплоть до того, что получив от клиента
> опцию 60 (Vendor Class Identifier) DHCP-сервер должен ему возвращать только
> значение опции 43 (Vendor Specific Information), в которой закодирован и
> TFTP-сервер, и имя файла-образа, а опции 66 (next-server) и 67 (filename) при
> этом должны игнорироваться.

Когда я это реализовывал, я решал общую задачу вычленения
DHCP-сообщений от пропагатора из общей массы трафика. Для этого можно
было снабдить сообщения опцией 60 (так, как сейчас в таске; так же
поступает dhcpcd), либо, например, опцией 77 (так делает iPXE).
Опцию 77 описывает RFC 3004; из этого документа легко сделать вывод, что
она вообще предназначена для самоидентификации со стороны
клиента либо категории его пользователя:

} DHCP clients implementing this option SHOULD allow users to enter one
} or more user class values.

Что касается трактовки «Servers that respond SHOULD only use option 43
to return the vendor-specific information to the client», посмею предположить,
что здесь по правилам грамматики английского языка подразумевалось
не разговорное «only» в начале предложения, а «серверам СЛЕДУЕТ заносить 
особенную информацию только лишь в опцию 43». Как-то оно менее абсурдно
в такой интерпретации звучит.

Большинство option-ROMs сетевых карт заполняет опцию 60 чем-то вроде
"PXEClient:ARCH:0007:UNDI:003010", или в последнее время
"HTTPClient:Arch:00016:UNDI:003001". Сервер, который им бы на это
отвечал тарабарщиной в опции 43, не был бы пригоден к использованию.

К тому же строчка вроде
«ALT-Propagator-20180601-alt1:Linux-4.9.99-std-def-alt1:amd64»
передаёт очень ценную информацию и совершенно бесповоротно описывает
вендора DHCP-клиента, а именно Alt Linux Team.

> я уже говорил, где у нас в сетевой загрузке совершенно точно нарушен
> стандарт и на что это влияет
Хоть я и не настаиваю, но прошу всё же посвятить в курс, ибо сейчас этого
нигде в непосредственной близости не задокументировано.
Связано ли это с root-path?

----- Forwarded message from bugzilla-daemon@altlinux.ru -----

Date: Thu, 24 May 2018 03:14:41 +0300 (MSK)
From: bugzilla-daemon@altlinux.ru
To: arseny@altlinux.org
Subject: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs

https://bugzilla.altlinux.org/34347
Component: Sisyphus/propagator

--- #13 Leonid Krivoshein <klark.devel@gmail.com> 2018-05-24 03:14:39 ---
(В ответ на комментарий №12)
> С пропагатором 20180423-alt1 сетевые карточки действительно подхватываются
> (спасибо, Леонид!), но проблема из сабжа всё же ещё не решена.

Проблему с нуль-маршрутом я действительно не решал, хоть и видел ваш патч,
поскольку мало что в этом понимаю. А что масштабно потестировали новую версию
пропагатора -- вам отдельное спасибо!

> Прошу желающих/заинтересованных потестировать, а тех,
> кто в ACL — одобрить таск №206842 с исправлением ошибки.

Предлагаю анонсировать максимально масштабно и в devel@, и в sisyphus@ хотя бы.
По предыдущему опыту желающих тестировать и ревьювить такое мало, если не
сказать "нет совсем". А в данном случае тестирование ещё более сложное,
поскольку нужны шлюзы. Код посмотрел и даже есть что сказать, но такие вещи
обсуждать лучше в devel@. Например, вот эту цитату:

Servers that respond SHOULD only use option 43 to return the vendor-specific
information to the client.

из RFC-2132 9.13 трактуют по-разному, вплоть до того, что получив от клиента
опцию 60 (Vendor Class Identifier) DHCP-сервер должен ему возвращать только
значение опции 43 (Vendor Specific Information), в которой закодирован и
TFTP-сервер, и имя файла-образа, а опции 66 (next-server) и 67 (filename) при
этом должны игнорироваться.

Но я не уверен в правильности такой трактовки RFC-2132/4361/etc и исхожу из
того, что вы лучше понимаете, что делаете. Георгию Курячему я уже говорил, где
у нас в сетевой загрузке совершенно точно нарушен стандарт и на что это влияет.


-- 
You reported the bug.

https://bugzilla.altlinux.org/e (basic email parameters)
https://bugzilla.altlinux.org/s (additional subscription)

----- End forwarded message -----

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Leonid Krivoshein]

28.05.2018 19:19, Arseny Maslennikov пишет:
> On Thu, May 24, 2018 at 03:14:39AM +0300, Leonid Krivoshein wrote:
>> По предыдущему опыту желающих тестировать и ревьювить такое мало, если
>> не сказать "нет совсем".
> Я готов содействовать по мере наличия времени.
> Нам в ACL пропагатора (пока его ещё не закопали) нужен кто-то, кто готов
> мониторить возможность сетевой загрузки дистрибутивов, без этого ошибки
> исправляться не будут. Насколько мне известно, текущий состав ACL просто
> сильно загружен.

Тоже могу поучаствовать в меру занятости, раз уж пришлось в него окунуться.

Справедливости ради: моё задание ревьювил rider@, тестировали перед 
пропуском в Сизиф и p8, в том числе, ребята из Обнинска. Корректная 
сетевая загрузка для нас актуальна, так что ещё одна площадка (ВМК МГУ) 
будет отличным подспорьем!


>> я уже говорил, где у нас в сетевой загрузке совершенно точно нарушен
>> стандарт и на что это влияет
> Хоть я и не настаиваю, но прошу всё же посвятить в курс, ибо сейчас этого
> нигде в непосредственной близости не задокументировано.
> Связано ли это с root-path?

Поставил в копию на баг #34889 (застолбил номерочек, первое сообщение и 
заголовок лучше сразу пропустить). Если коротко, то да: next-server и 
root-path.

Кстати, в этом баге я начал про IP и путь к ISO. И так совпало, что в 
баге #34347 вы их тоже прописываете руками в загрузчике, чтобы 
воспроизвести. Тогда как alterator-netinst "из коробки" их не 
прописывает, полагаясь на DHCP-сервер. И там же вы говорите о 
недоступности для вас конфигурации DHCP-сервера.

Это принципиально: либо вы передаёте эти опции пропагатору через 
загрузчик, либо он их ловит в ответах от DHCP-сервера. В случае 
PXE-загрузки -- да, их можно прописать на том же сервере в default, но 
это всё-таки совсем другой use-case.


-- 
Best regards,
Leonid Krivoshein.

Re: [devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Leonid Krivoshein]

28.05.2018 19:19, Arseny Maslennikov пишет:
> Пользуясь случаем, всё-таки прошу одобрить таск 206842, исправляющий
> баги 34347 и 34320. Если код слишком плохой даже для пропагатора,
> я поправлю, мне не жалко. :)

Видимо я не спец в нулевых маршрутах, поэтому мало чем могу быть 
полезен. :) Но строчка assert((sz - sp) == 1); вызвала спортивный 
интерес: как поведёт себя приложение с PID=1, если утверждение окажется 
ложным, с учётом того, что в опциях сборки нет NDEBUG. Один не 
закомментированный assert() в init.c есть, это будет второй.


-- 
Best regards,
Leonid Krivoshein.

Re: [devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Arseny Maslennikov]

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

On Wed, May 30, 2018 at 04:34:23AM +0300, Leonid Krivoshein wrote:
> 
> 28.05.2018 19:19, Arseny Maslennikov пишет:
> > Пользуясь случаем, всё-таки прошу одобрить таск 206842, исправляющий
> > баги 34347 и 34320. Если код слишком плохой даже для пропагатора,
> > я поправлю, мне не жалко. :)
> 
> Видимо я не спец в нулевых маршрутах, поэтому мало чем могу быть полезен. :)
> Но строчка assert((sz - sp) == 1); вызвала спортивный интерес: как поведёт
> себя приложение с PID=1, если утверждение окажется ложным, с учётом того,
> что в опциях сборки нет NDEBUG.

Как и любое другое, завершится со всеми вытекающими последствиями (я
этим вопросом тоже задался, когда готовил исправление).

С другой стороны, код, в котором такое утверждение под assert() ложно,
содержит логическую ошибку и должен быть исправлен до попадания в сизиф.
Именно так его применяют, например, в (извините) systemd — деймон, который
падает со скрипом, да ещё сообщает место, куда смотреть, лучше, чем
"работающий" деймон с утекающей памятью, или такой, который просто
продолжает творить некоторую известную глупость, в то время как разработчик
не знает горя. Да, даже если это PID 1, в таком случае — тем более.

assert() в нашем случае упрощает тестирование: увидя, что "система
загрузилась" (пропагатор сделал всё необходимое), мы точно знаем, что
определённой глупости не будет, вместо того, чтобы на это надеяться.

> Один не закомментированный assert() в init.c
> есть, это будет второй.
> 

Вообще-то можно было реализовать так, чтобы не нуждаться в дополнительной
проверке.
Коли assert() здесь считают инструментом для слабых духом, я подумал и
решил переписать этот сегмент — тег 20180606-alt1.
Так что пока второго assert() не будет :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Leonid Krivoshein]

11.06.2018 13:11, Arseny Maslennikov пишет:
> On Wed, May 30, 2018 at 04:34:23AM +0300, Leonid Krivoshein wrote:
>> 28.05.2018 19:19, Arseny Maslennikov пишет:
>>> Пользуясь случаем, всё-таки прошу одобрить таск 206842, исправляющий
>>> баги 34347 и 34320. Если код слишком плохой даже для пропагатора,
>>> я поправлю, мне не жалко. :)
>> Видимо я не спец в нулевых маршрутах, поэтому мало чем могу быть полезен. :)
>> Но строчка assert((sz - sp) == 1); вызвала спортивный интерес: как поведёт
>> себя приложение с PID=1, если утверждение окажется ложным, с учётом того,
>> что в опциях сборки нет NDEBUG.
> Как и любое другое, завершится со всеми вытекающими последствиями (я
> этим вопросом тоже задался, когда готовил исправление).
>
> С другой стороны, код, в котором такое утверждение под assert() ложно,
> содержит логическую ошибку и должен быть исправлен до попадания в сизиф.
> Именно так его применяют, например, в (извините) systemd — деймон, который
> падает со скрипом, да ещё сообщает место, куда смотреть, лучше, чем
> "работающий" деймон с утекающей памятью, или такой, который просто
> продолжает творить некоторую известную глупость, в то время как разработчик
> не знает горя. Да, даже если это PID 1, в таком случае — тем более.

Разница между демоном, который крутится в stage2 месяцами, и инитом с 
PID=1 в initramfs/stage1 очевидна. Для демона утечки памяти -- "мина 
замедленного действия", пропагатор же ими весь напичкан (был?). И 
ничего: всё "прощается" и забывается при переходе в stage2. Если же 
упадёт инит, в журнал уже ничего не попадёт: последнее, что мы увидим в 
консоли -- хвост от паники ядра. Мы даже не поймём, что случилось и почему.


> assert() в нашем случае упрощает тестирование: увидя, что "система
> загрузилась" (пропагатор сделал всё необходимое), мы точно знаем, что
> определённой глупости не будет, вместо того, чтобы на это надеяться.

Проверялись длины строк (адреса), зависящие от конкретной машины. В этой 
ситуации assert() слишком жестковат в качестве средства отладки. 
Поскольку надеяться можно будет на вашей машине, но так ли это будет на 
других?..


>> Один не закомментированный assert() в init.c
>> есть, это будет второй.
> Вообще-то можно было реализовать так, чтобы не нуждаться в дополнительной
> проверке.
> Коли assert() здесь считают инструментом для слабых духом, я подумал и
> решил переписать этот сегмент — тег 20180606-alt1.
> Так что пока второго assert() не будет :)

assert() безумно полезная вещь, но из пропагатора я бы его везде 
повыкидывал... от греха подальше. В этом пакете даже ключик NDEBUG будет 
"миной замедленного действия". И хорошо, что поменяли этот фрагмент, 
вопросов будет меньше.


-- 
Best regards,
Leonid Krivoshein.

Re: [devel] [bugzilla-daemon@altlinux.ru: [Bug 34347] Странный временный нуль-маршрут ломает сетевую rootfs]

[Anton Farygin]

28.05.2018 19:19, Arseny Maslennikov пишет:
> On Thu, May 24, 2018 at 03:14:39AM +0300, Leonid Krivoshein wrote:
>> По предыдущему опыту желающих тестировать и ревьювить такое мало, если
>> не сказать "нет совсем".
> Я готов содействовать по мере наличия времени.
> Нам в ACL пропагатора (пока его ещё не закопали) нужен кто-то, кто готов
> мониторить возможность сетевой загрузки дистрибутивов, без этого ошибки
> исправляться не будут. Насколько мне известно, текущий состав ACL просто
> сильно загружен.
У нас есть механизм тестирования через специальную службу. Можете 
сделать test-only задание и отдать на проверку sotor@altlinux

Это без вычитки кода, просто проверка функционала. Если на что-то нужно 
обратить внимание при создании стенда - напишите и это тоже.