Dear viewer, these subtitles were generated by a machine via the service Trint and therefore are (very) buggy. If you are capable, please help us to create good quality subtitles: https://c3subtitles.de/talk/104 Thanks!
OK. Let's see if things start getting
better or worse.
Yeah. Thank you very much for coming.
I'm really happy to see such
a big crowd that
is seems to be interested in this topic.
I am a Linux kernel developer,
and so obviously I
prefer to speak about the Linux kernel.
Nonetheless, most of the things I want
to say are pretty generic, so you can
apply them to whatever open source
project. It's more or less
some observations about patches and how
they are dealt with in open source
projects.
So while this talk is kind focused,
try to see the general bits
in that.
I have a few assumptions about you
because I think we're I have a quite
an educated audience here,
so I will
hope you have some experiences in hacking
with with source codes and are
somehow able to read a little bit
unify this because I will have some of
them in my slides.
And one thing I want to mention is about
your devices.
I know it's pretty complex, getting
pretty complicated, having mobile phones
and phones and all this with you.
But please make sure that during talks
they are muted
if they are ringing during the talks.
This is at least irritating for the
speaker and I'm also
sometimes in the audience, and I thought
this was pretty annoying during the first
day, so please mute them.
One disclaimer I
am, of course, as the title suggests that
I'm going to present a
bad and ugly patches, but
I don't really want to bashing that does
not mean that I think the people who
created these patches are bad
and ugly.
Not necessarily.
There are lots of causes why, why patches
can be in such a state, and surprisingly,
this is not too different from
what happened in open source projects
and companies.
There might be big companies who have
policy issues about not doing open
source, but there are also
smaller companies who'd like to
contribute and face the same problems,
like I'm not sure developers
or open source projects.
Very often it's just a too high workload,
or they have to create a patch
as a side effect to focus on another
thing, and then they don't have time to
clean it up while they work on some
hardware. Get a new gadget and then the
old gadget is not interesting anymore,
and they couldn't finish the Old Patch.
All of this stuff happens, and
it's one reason why why we have patches,
which needs a bit further life
or a bit more attention.
And yes, there are also bad coders, but
we won't talk about those patches
too much here.
So this is not about bashing
yet. This is about improving the
situation.
I want to mention a few projects
I encountered.
First one is Osmo.
Com B-BBEE, which was pretty surprising
because they themselves
don't use a Linux kernel.
I have.
Yeah. Well, this is one of their Motorola
phones they're using.
It's not powerful enough to use a Linux
kernel.
Still, I was for a short time
a bit involved in the development,
and I came when I was reading
the mailing list. I found it.
Well, this is an excerpt of a mail
thread, and there was one person
saying, OK, I've got this USB cable,
which he needed to to connect
to the phone, and it doesn't work in
the Linux kernel.
Being a hacker, he was he
was able to fix the Linux kernel himself
because he just found out, OK, it was a
cable which has just in you use b ID.
So he modified
the driver to recognize
this and use video so
he could work and focus and follow
his main focus working with the phone.
He
sends the patch to the mailing list, and
you can see it's it's pretty simple.
It's just adding two lines lower line
as he defines the new
idea and put the new ID in the
the table for the driver so it gets
recognized.
It's pretty simple.
You can't really do much
wrong with that.
And then I
asked him, Hey, well, what do you do
incident intend to send this to mainline?
Because I mean, for you, it was just
a short exercise.
But other people not having this
knowledge, they might be really happy if,
of course, the hardware gets detected
by default.
And yeah, he was interested,
but he had not thought of this before.
So he just needed a little input
and wanted to send this upstream, and
it was pretty s just a simple
patch. It didn't need very much.
What is, of course, required is a
patch header having a description
of what this patch does and
you need this famous signed off line.
So it's a real name and your address.
So
in case something is wrong with the
patch, everyone knows who
to contact
this.
Adding a new ID is a pretty simple
process, which happens very often in the
kernel. So to be honest, it's a very,
very descriptive
patch description text.
It could have been shorter, but even this
descriptive version, as you can guess,
should not take more than
one minute or so to type
in.
And this guy
added the patch headers send it in, and I
think between sending the patch and
applying to the USB
tree was a matter of a few hours.
So, yeah, patches
can take quite a while, but sometimes
it's pretty fast.
And from personal communication,
I recall that this
guy was kind of happy to have his first
call, as was his first kernel patch
accepted. And so, yeah,
he felt good about it.
So this was an example, good example
of the patch flaw, how it went out
of the original project and
went to where it belongs to to the
upstream project, namely the Linux
kernel.
If you want to do that
and don't know these tools yet,
I wouldn't try to want to do a short
recommendation.
The first one is the famous
Czech patch Perl script,
which checks for typical flaws.
Please use it even if you're an
experienced kernel hacker.
It's so easy to to get some whitespace is
wrong, and so just run over it and
have your patch checked.
And what's really useful is if you
don't know who is who in the kernel
community and who to send patches to
theirs to get maintainer Perl script.
Just pass your patch file
to that script, and it will print a list
of email addresses who
to whom you should.
You should send the patch to
if you work more with them.
It makes sense to have a look at the
command line options, but meanwhile
they have pretty sane defaults.
So if it's just if you
use it occasionally, you just use them
like here without any options and they
will give you reasonable
results.
So that was what I think was
a good example.
Somebody hacked something.
It was worthwhile adding to the Linux
kernel, and she took the
necessary steps to do that.
The next thing is
the zippered that to kernel
zipper that too was, oh yeah,
I think in 2007
or something, it was a small
device intended for instant
messaging everywhere.
So it's a
small p h of 70 base
arm based mini computer.
I think this size, it has this
small display, keyboard, Wi-Fi.
Well, there's lots of devices
like this around, but a few
years ago they were sold pretty
cheap for like ten or fifteen dollars and
you could get heaps of them and
they are quite easily hackable,
which makes them interesting devices
and they
have some support
in the mainline kernel, these
devices.
But it's a bit rough.
So there's an extra kernel around
in the net to to make it, to make
it run more smoothly
and in that kernel.
I found this patch
which where you can see if you read the
description, mentions that
if it is not applied,
the device will crash if you use
frequency scaling.
And of course, you want to use frequency
scaling because it's good for battery.
And this is why I think it's a bad patch
because it's not in mainline.
I I really want to be Linux,
the kick ass operating
system that you can if you want to hack
something and be creative with a device.
And I want the Linux kernel to just
grab it and work is as good as possible.
So you don't have to spend on on fixing
things, but concentrate on what you want
to do with the device.
So crashing randomly is really not
not what I want the Linux kernel
to see.
That makes it, in my opinion, about
Patch. It should have been upstream.
Again, no bashing.
This guy has found other issues
while working on the on this
zip it's device and got
quite of them upstream.
But I think after some time he went to
the next device or whatnot, so this
patch is somewhat stalled.
Yet it would be nice to have upstream if
you look at the patch.
It's mainly about changing
one entry in the table you see here
a row of once, which means use of fast
boss mass method.
We don't need the details of what this
means, and here's a row of zero,
so that means disable that.
So we have a stable, stable
system.
So not not really rocket science.
What does it need in this case to
get this patch upstream in case you
won't? Let's assume you bought the device
and you want to prepare a good kernel for
that and you find the patch and
one.
My hope is that that you also get
the idea Hey, wait, what's not upstream?
It should be there. So what is needed to
do that?
First one, of course, check if it's
still needed.
Mostly the those kind of kernels
are a bit older.
I think this latest version
from the Zip It archive is
3.7.
Remember, we're approaching three point
thirteen, so it's probably already two
years old.
So rule number one, check if it's still
needed,
it might be that it found a similar
patch, a better patch or whatnot
has already found the
way upstream.
If you if you search for the patch, if
it's if it has been submitted already,
which is also a good idea, then
you'll find out OK, it has been submitted
already, but there were issues with it.
And what this patch does,
it disables fastballs
for all peaks, 270 balls,
which sounded a bit too aggressive
because for most
other peaks, H7 keyboards, it worked
perfectly.
So what was more or less requested for
this patch is to to extend it, to make it
more optional, to say, OK, I have
this part which is known to not work.
Please disable it now.
That's not a lot of code, but it has to
be done,
and that would be a useful option because
I quit. I'd be lying if the peaks
to 270 would be a state of the
art processor. It's pretty old by now,
but it has been deployed quite
often. So if you're buying cheap,
old, hackable devices,
you still have chances that it might
have that associate.
And then if such a patch is applied
upstream and another person discovers
I have random crashes, then it might
be. Then this person might easily find,
Oh, OK, there's another bot which
disables fastballs.
Maybe I need to do that too.
And a lot of engineering
debugging effort is saved
because otherwise this person would have
to do this again.
And random crashes are really
pain in the ass to D-backs to
debug. So that would be a good thing.
So another thing would be nice
here to extend the patch
tested on real hardware.
Does this things that I want updates
to the patch header slightly?
Send it upstream and be
around for questions?
Might be that people have other
issues raised. Maybe it goes in
without any complaints.
Just be around and don't disappear
like the original patch also did
for probably valid reasons.
I think that would be
a way how to make this that
patch a good patch, because after that,
it would be upstream.
This is one example of
of Petraeus you can find in
in repositories like a zip, it's
that to Colonel.
Open W.r.t.
They in the past,
meaning really a few years ago, they had
a reputation
of not having a lot of interesting
patches and not upstream.
Then that has really, really changed.
They have become very good at upstream.
Yet they have really a lot
of patches and
not enough manpower to
to upstream them all on their own.
Some maybe craft, some maybe interesting.
So they
have the they need support for that.
So if you happen to work
on open w.r.t and
have a kernel with its with which
is patched and might
be worth looking, OK, what patches do
I have? And is that something was up
and said some packaged patches?
I searched for for some examples.
This is OK.
An ugly patch, and
I want to extend my disclaimer.
There are some ugly patches in the open
w.r.t repository.
Not necessarily.
All of them are ugly, but for
educational reasons.
I concentrate more on the ugly ones.
Now, please don't get the wrong
impression of about open w.r.t.
And here you see that
a patch which simply erases a data
structures initialize it to zero.
And what I think, what
really makes this patch ugly?
There's no patch description, and there's
no patch, although
this is a pretty simple patch, so it's
not that bad, but we will see
that it can go a lot worse.
So this is one thing which really needs
to be fixed, and
it's kind of a trivial patch because
the data structure is
located on the stack and then
some values are initialized, and then
it's passed on to some Linux
kernel function and to
ensure that all the other unused fields
are there is good programing exercise.
So from a simple
review, this patch makes
perfectly sense and should be upstream
to.
What it needs to be
to be a good patch is check
if it's still needed, I can tell you it
is still needed.
I looked it up already.
Look for improvements.
Yeah, well, it's a trivial
patch, but still I would replace the
correct backslash.
Zero was a plain zero.
I think it's more readable, but
that's your decision if you take over the
patch. Such things are your decisions
signed since it's
fixing a programing flaw, waiting
on programing flaw with an easy fix.
There's an onus on initialized data
structure and initializing it zero.
It can be argued if you can leave out
the test on real hardware because it's so
simple.
Some people argue every patch should be
tested on real hardware.
I leave that up to you.
If somebody would send
it to my subsystem, I would accept
it as is
because I think it's obvious
what really needs to be done.
Of course I said before and a patch
header, which for the Linux kernel and
quite meanwhile quite some other
subsystems, is adding a patch
header with one line
description and then a multi-lane
description and the signed off by line.
Once you get used to that, it's done in
really in a minute and then send it
upstream.
As mentioned before, if you use a script,
get maintainer, it
will present you home to send it to and
this is makes it pretty easy.
And then we have a clean
code in the Linux kernel.
The open w.r.t project loses the
patch, which it has to maintain,
and you will get credited
for having a Linux kernel patch if
that gives something to you or you have a
better operator, better operating
system if that gives something to you.
Next, patch from
open W.r.t
is somehow a little bit
similar to the
USB patch because what it basically does,
it adds detection for some flash
types and
all flash types, and
that defines the IDs
and and extends
the array of parameters.
These flash chips have
since we want to support as many hardware
as possible in the Linux kernel.
This should be off stream, of course, as
well.
So what needs to be done here?
Check if it still applies to the recent
kernel.
You should probably have guessed that.
And here also, I can tell you, yes, it
still applies to the recent kernel.
So we want we want this up.
We can upstream this and we want upstream
this after we checked.
If it's the best solution.
This is.
And encouragement to to work,
to not blindly send patches upstream
without knowing what you're doing, son,
but also to to work into the
topic a little bit because
you could find out that most, if
not flash chips, have
something called something called see if
I common flash interface.
And usually this is enough
to auto detect so you don't have
to to add anything
to the probe function at all.
So if you have the hardware, you could
check if it still works without the patch
and the CFI interface is enough.
You could find out then that this if I
interface in that flash chips is broken
and you really need the patch.
I also added some some
flash chips because they advertised
some something, which they
did not do in reality.
And so we had to to add
this manual detection instead of the
automated one.
And this is something which could be done
here as well.
It's fun hacking, in my opinion.
I like hacking with with embedded devices
and getting chips to work and and
reading registers and stuff like that.
And once you did all this
again at the patch headed here,
we really should test
that on real hardware.
And this is, I think, too advanced
for for an educated guess.
So make sure it works on hardware.
And again, send it upstream and be around
to answer questions.
That sounds like a long list, but
I don't think it's really that long.
Depending on your experience, this might
be an hour or one or two evenings
of hacking and fiddling around
reading data data sheets.
And then we have support for new flash
cards, slow
next bought you, you
might support them automatically,
might be good.
This is a patch
which I've seen,
in my opinion, is really only because I
don't have much knowledge about this
subsystem, I don't know.
My experience is a driver's
device. Drivers, especially eyes,
see where I'm the maintainer of the
subsystem.
OK, this is about people.
Oh, I don't know much about the internals
of that one.
And what I see here is
that
there are two places where some memory
is allocated and the patch
extends the amount
of memory allocated.
So my first guess, it
wants to prevent a buffer overflow, which
I think is very, very important.
And here it's really for me, it's really
annoying. There's no patch description.
I don't know if this is the proper fix.
Is this a hacky workaround?
Or is there something open w.r.t
specific because they hacked something
out somewhere?
And in order for me to find
out if this is a bug fix, I basically
have to do all the engineering work again
to find out what, what the status of this
patches.
So this is why I think this patches is
extremely obvious
when I work commercially.
It's also true that sometimes
you get patches exactly like this.
You have no idea what they were, where
they came from. You can't.
There's nobody took what you could
contact and you have no idea if this is
important to a rubbish.
So what do you need to make
with this patch check if it still
applies? Maybe someone else has
already fixed the same problem and has
a proper commit message
so you could understand what's going on.
I can tell you it still applies to the
current kernel. So if it's an issue,
if it's a buck, it's still open
check. What I just mentioned, check if
it's a workaround, the custom case
or an important bug fix on
C or related to that check if
the patch has dependencies or if it's
a standalone doc fix.
And if you found out that it's something
which should be upstream again at
Patch, however, I leave it up to you.
If you're tested on real hardware, send
it upstream and be around
to answer questions.
And yeah,
if it's really about fixed and
then it fixes a buffer overflow and it
should be so upstream.
So if any one of you is familiar
with that PGP interface and
actually knows if there is a bug fix,
please hurry up.
What I also want to mention, this is not
black and white, not it's not all or
nothing.
I know that
it isn't the same.
What I said in the beginning might apply
for you. You just stumble over the patch,
but you have something else in your
focus. So you don't really want to spend
time too much time on upstream a patch
you're not mainly interested in.
But that's what what I think where
also where small steps end up.
So if you just happen to know
that this is a bug fix, or it can say
something about the patch which has
useful information just to
add to the patch pattern
and send this a patch to open w.r.t
that won't make the patch immediately go
upstream to the Linux kernel, but it
will help other people to understand
what's going on with that patch.
And I really, really think that just
adding a patch headers is definitely
worth an commit and not only changing
code.
Sometimes people ask me how to
get into Linux kernel development,
and I think this is a
good, good way to start an open
w.r.t supports a lot of devices
and a lot of them can be bought for very,
very little money these days on
an auction site.
We auction sites
and it's a good way to get into kernel
hacking by just buy one such cheap
thing get the version,
which is kernel version, which is inside
open. You actually get it running and
then try to do to upgrade the kernel to
something more recent, even the most
recent.
This can be somewhere which
is sometimes needed to test one
single patch. Then you find out, Oh no, I
have to port the whole platform.
This can be extra work
and it can make you go to work, go away.
But if you're interested and said, Hey, I
want to learn something about Linux,
especially embedded Linux kernel
development, that's a really good way to
learn things.
So I can recommend that
what I have not talked about,
there are nightmare patches too.
You usually get them from chip vendors
May one.
One thing where you can easily detect
into like one or two megabytes in size
and patch everything at once.
So with that, you have really a
different set of problems.
You split up into
separate chunks, decide on each chunk of
its use, useful or not.
This is a nightmare, and they often
contain useless changes fixing
whitespace, adapting to some personal
coding style, making everything look
different for nothing.
But let's start with the lower hanging
fruits from which I hope to
hopefully showed you some
and hopefully presented you some ways
to do that.
So I hope that
more patches are coming upstream.
And one thing I also want
to mention if you create patches.
Think about sending them upstream in the
first send, make some good patches or
at least make them not ugly patches
and describe them to some extent.
Some people later can pick them up and
do something more useful with them.
Yeah, that was
the contents.
Talk in half an hour, and that's what I
wanted to give you
from my experience.
I.
I really wish you a happy hacking,
because in my opinion, it's great fun,
and if you have questions, I think we
have one minute left
and above it, feel free to meet
me somewhere here at the conference.
I will be here till the end or write
me an email. I'll be around.
And if you leave the room, please try to
be quiet so we can hear the questions if
there are any.
Does the internet have questions?
No, somebody's here.
No, no.
OK.
Right, thank you very much.
We already gave Motorola
One
round of applause.