Bug 5342

На данный момент начал продвижение следующих патчей:
1. mandatory reading (ветка etersoft-mandatory-reading).
2. getflk problem (etersoft-getflk).
3. direct problem (etersoft-direct) - для direct+nobrl (direct, без nobrl уже в upstream).

Патчи рассматриваются, ведётся обсуждение.

http://patchwork.ozlabs.org/project/linux-cifs-client/list/
обсуждение можно посмотреть здесь.

Патч "direct problem" принят: коммит 810627a013163cd294762d57c0ea2ec055ffe4f6.
По двум другим патчам ведётся обсуждение, причём патч "getlk problem" уже готовы принять, - переоформил их согласно требованиям (стиль и подробное описание) и выслал заново.

Отправил ещё один патч, связанный с потерей блокировки в главном процессе после завершения порождённого процесса - блокировку нельзя снять, но она есть.

По предыдущим патчам: патч 1 вызвал дискуссию о целесообразности использования данного подхода вместо простого включения опции direct - буду пытаться тестами выяснить улучшится ли производительность; патч 2 принимают, но пока его нет в upstream.

Патч getlk в upstream - коммит f05337c6ac48d19d354e0640a8eb8fc884f82bcc.

Патч, связанный с потерей блокировки принят в upstream:
коммит 2c964d1f7c87eb71f7902111cd7c8fbba225e4b6.

Поднял тему обсуждения проблемы кеша в CIFS в рассылке разработчиков. По результатам тестирования в баге 5442 можно сделать вывод, что код, который следует спецификации (сборка 4.5.1.4) не ломает локику работы приложений, но использует кеш при одиночном доступе к файлу.

Обсуждение начато, можно наблюдать на http://news.gmane.org/gmane.linux.kernel.cifs - тред CIFS data coherency problem.

Отправил ещё один тест, подтверждающий проблему. Так же описал проблему с одной шарой, подмонтированной в несколько директорий - Oplock break возвращается сервером только в одну.

Заметил, что результаты работы с оплоками отличаются для Samba 4.0.0-alpha11 и Samba-3.4.7 (Ubuntu).

Samba-4:
1) client1 открыл файл с oplock ex;
2) client1 записал в файл4
3) client2 открыл файл с oplock level II, на client1 oplock стал тоже level II;
4) client2 записал в файл и получил oplock level None;
5) client1 открыл файл через другой дескриптор, получил oplock level II.

Samba-3.4.7:
1) client1 открыл файл с oplock ex;
2) client1 записал в файл:
3) client2 открыл файл с oplock None, на client1 oplock стал тоже None;
4) client2 записал в файл;
5) client1 открыл файл через другой дескриптор, получил oplock None.

Во втором случае поведение корректное, в первом нет, так как client1 не получает  Oplock break to None после того, как client2 записал в файл.

Спецификация MS-CIFS не содержит точного упоминания по поводу установки поля NewOplockLevel в Oplock Break Acknowledge. Сделал вывод, что оно просто не используется, продолжаем обсуждать с разработчиками проблему несбрасывания сервером Samba 4.0.0-alpha11 оплоков level II для всех открытых дескрипторов.

Подготовил и отправил патч по баги 5442 в апстрим. Ждём результатов.

После обсуждений пришли к выводу, что надо разбить патч на три независимых патча:
1) пропуск при чтении кеша если мы не имеем оплока на запись или чтение;
2) установка кеша в неактуальное состояние после закрытия последнего дескриптора;
3) запись на сразу сервер если у нас нет оплока на запись.

Последний патч вызвал замечания по поводу производительности, собираюсь поправить.

Отправил первый и второй патч в upstream. Начал работу над третьим.

По первым двум патчам завязалось обсуждение, в процессе которого выяснилось, что стоит добавить новую опцию монтирования для данного поведения. Добавил опцию strictcache, поправил патчи (1-ый и 2-ой) и отправил заново.

Поправил патч из http://bugs.etersoft.ru/show_bug.cgi?id=6227#c9, отправил в рассылку.

Патч из http://bugs.etersoft.ru/show_bug.cgi?id=5342#c17 принят в upstream.

Поправил патч из баги 4875 и создал патч для upstream. Отправил.

Во время обсуждения патча из баги #4875, решено было переписать код, устанавливающий соединения с сокетом в cifs. Сделал это и отправил патчи на обсуждение.
1) объединил функции ipv4_connect и ipv6_connect в одну, убрав много дублирующего кода. Отделил код, работающий с RFC1001 серверами (порт 139).
2) добавил отдельную функцию match_port, в которую прокинул наши исправления из баги #4875.

Поправил патчи, что отправил утром. Переотправил.

Патчи вызвали дискуссию, я попытался подробнее объяснить проблему текущего кода. Вот письмо, что я составил и отправил:

"Let's predict the following situation:

we have server with two smbd's: 1th on 445 port, and 2nd on 5000 port.
At first user mounts 2nd smbd with specifying 5000 port. Then it tries
to mount 1th without specifying any ports. If we consider your logic,
our client looks for an any existing connection, finds connection to
5000 port (2nd smbd) and mounts it. So, as the result, user thinks
that it successfully mounted the 1th share, but it is the 2nd - it is
not right.

If we should follow mount.cifs spec, I think we need to do:
1) if user doesn't specify a port we at first try to match a
connection with 445 port, then if we are not successful - with 139
port. If there are no connections with these ports, we return an
error.
2) if user specifies a port, we should try to match a connection only
with this port. If no connections with this port, we should return an
error.

I think that it is real problem. I faced with a real application
freenx server that uses CIFS for mounting many shares on different
ports (freenx clients want mount to mount its own share through ssh
port forwarding - many smbds with non-standard ports and local smbd
with standard port) on localhost.

You can try the following to reproduce the bug:
run smbd's on localhost and smbserver_host (another).

On the first console:
ssh -L5000:smbserver_host:445 smbserver_host
On the second:
mount -t cifs //localhost/share ~/test -oport=5000
mount -t cifs //localhost/share ~/test2

As the result, we can see that the second call connects to
smbserve_host rather than to localhost and return without any error -
so, user doesn't know that it has mounted smbserver_host instead of
localhost.

Even more, if we unmounted and mount them vice versa:
mount -t cifs //localhost/share ~/test2
mount -t cifs //localhost/share ~/test -oport=5000

it will be ok - two different connection to two different share, but
(!) if we losing the connection to port 5000, today's code reconnects
with share to localhost - wrong again! My patch number 1 fixes this
situation too.

I am ready to provide changes to the patches I posted later according
to mount.cifs spec as soon as we find an agreement in all the things"

Получил комментарии по первому патчу (который делает коде рефакторинг в организации установки соединения с сервером). Собираюсь сделать следующее:
1) сделать код функции ip_connect независимым от порта и перенести логику выбора порта для соединения в другую функцию;
2) исправить некоторые баги и неточности в данном патчи.

Поправил оба патча - теперь логика следующая:
1) если у мы указываем порт, то используется только он.
2) если мы не указываем порт, то сначала пробуется 445 порт и если не удаётся - 139 порт.

Так же при поиске существующего соединения мы:
1) если указан порт, то ищём с ним.
2) если не указан, то ищем либо 445 либо 139 порт.

Отправил в рассылку. В случае успеха, дальше прокину эту логику в наши ветки.
(сейчас, начиная с 4.5.5, наш код отличается тем, что при поиске существующего соединения если порт не указан, мы ищем только 445 порт). Таким образом в случае 139 порта мы имеем на каждое соединения отдельный сокет.

Сделал патч для mount.cifs man страницы, изменив описание логики работы опции 'port=', согласно описанному выше.

Отныне тут буду только регистрировать результат продвижения патчей. Процесс их разработки отмечается в баге http://bugs.etersoft.ru/show_bug.cgi?id=6517.

Выяснилось, что патчи связанные с ip_connect потерялись как-то. Переотправил.

Обсудил текущие патчи прошедшие ревью (helper, port) - на днях ожидаю их появление в апстрим. Также выяснилась проблема в патче writepages из группы патчей описанно тут (http://bugs.etersoft.ru/show_bug.cgi?id=6633#c1).

Патчи группы port:
http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=a8b87002445df0cb0940779bb49bfba463973e01
http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=a6e8f418503d74d8822461318b2cb9b3a392fd58

Переоформил патчи группы helper с полученными reviewed-by тэгами и отправил Стиву Френчу.

Патчи группы helper в апстрим:
http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=6960be06f8c573ae4a27ddfdf611ff8ac0b90c12
http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=5f39af77bee7f5471a9bcae9c232c7cb509c536f

Обновил master на git.eter - патчи группы helper и port в главной ветке ядра.

После личной беседы со Стивом Френчем патч, исправляющий разбор опций монтирования, включён в master на git.kernel.org. Так же обсудили патчи, решающие проблему обработки ошибок к логиче валидации айнода - первый патч принят (из http://bugs.etersoft.ru/show_bug.cgi?id=6940#c24), второй требует большего осмысления и доработки.

Отправил патч, исправляющий ошибку в разборе опций монтирования d stable ветку (http://bugs.etersoft.ru/show_bug.cgi?id=7055#c15).

Патч из http://bugs.etersoft.ru/show_bug.cgi?id=6517#c44 в основной ветке ядра.

Патчи bugfix'ы для режима разделённого суперблока в апстрим:

http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=b2e5cd33b598fb496b9366c445bd77c801efabb8
(http://bugs.etersoft.ru/show_bug.cgi?id=6517#c45)

http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=641a58d66d086327042e9d73c6606fd02c8f067c (http://bugs.etersoft.ru/show_bug.cgi?id=7055#c25)

Отослал в рассылку патч для man mount.cifs, исправлющий описание опции wsize согласно новому режиму разделённого суперблока.

Отослал в рассылку патч для man mount.cifs, добавляющий информацию об опции монтирования rwpidforward.

Патчи режима разделённого суперблока и проброса пида в основной ветке ядра: http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commit;h=40efeb4d0bb1993c3c10baff9b7d86839f99171e

Патчи из комментариев 37 и 38 в апстрим.

Бага себя исчерпала. Дальнейшее продвижение патчей будет регистрироваться в одноимённых багах по их разработке.